From 5496295aefe86995e41398b0f76de601308fc3f5 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 11 Apr 2018 16:47:35 -0700 Subject: [PATCH 1/4] nfp: ignore signals when communicating with management FW We currently allow signals to interrupt the wait for management FW commands. Exiting the wait should not cause trouble, the FW will just finish executing the command in the background and new commands will wait for the old one to finish. However, this may not be what users expect (Ctrl-C not actually stopping the command). Moreover some systems routinely request link information with signals pending (Ubuntu 14.04 runs a landscape-sysinfo python tool from MOTD) worrying users with errors like these: nfp 0000:04:00.0: nfp_nsp: Error -512 waiting for code 0x0007 to start nfp 0000:04:00.0: nfp: reading port table failed -512 Make the wait for management FW responses non-interruptible. Fixes: 1a64821c6af7 ("nfp: add support for service processor access") Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c index 99bb679a9801..2abee0fe3a7c 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c @@ -281,8 +281,7 @@ nfp_nsp_wait_reg(struct nfp_cpp *cpp, u64 *reg, u32 nsp_cpp, u64 addr, if ((*reg & mask) == val) return 0; - if (msleep_interruptible(25)) - return -ERESTARTSYS; + msleep(25); if (time_after(start_time, wait_until)) return -ETIMEDOUT; From bc05f9bcd8cb62f935625850e535da183b4a07c0 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Wed, 11 Apr 2018 16:47:36 -0700 Subject: [PATCH 2/4] nfp: print a message when mutex wait is interrupted When waiting for an NFP mutex is interrupted print a message to make root causing later error messages easier. Signed-off-by: Jakub Kicinski Reviewed-by: Dirk van der Merwe Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/nfpcore/nfp_mutex.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_mutex.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_mutex.c index f7b958181126..cb28ac03e4ca 100644 --- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_mutex.c +++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_mutex.c @@ -211,8 +211,11 @@ int nfp_cpp_mutex_lock(struct nfp_cpp_mutex *mutex) break; err = msleep_interruptible(timeout_ms); - if (err != 0) + if (err != 0) { + nfp_info(mutex->cpp, + "interrupted waiting for NFP mutex\n"); return -ERESTARTSYS; + } if (time_is_before_eq_jiffies(warn_at)) { warn_at = jiffies + NFP_MUTEX_WAIT_NEXT_WARN * HZ; From 0b1a989ef5a751b5992842d1934e22de861a848e Mon Sep 17 00:00:00 2001 From: Pieter Jansen van Vuuren Date: Wed, 11 Apr 2018 16:47:37 -0700 Subject: [PATCH 3/4] nfp: flower: move route ack control messages out of the workqueue Previously we processed the route ack control messages in the workqueue, this unnecessarily loads the workqueue. We can deal with these messages sooner as we know we are going to drop them. Fixes: 8e6a9046b66a ("nfp: flower vxlan neighbour offload") Signed-off-by: Pieter Jansen van Vuuren Reviewed-by: Jakub Kicinski Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- drivers/net/ethernet/netronome/nfp/flower/cmsg.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c index 3735c09d2112..8ad857eb89c6 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c +++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c @@ -258,9 +258,6 @@ nfp_flower_cmsg_process_one_rx(struct nfp_app *app, struct sk_buff *skb) case NFP_FLOWER_CMSG_TYPE_ACTIVE_TUNS: nfp_tunnel_keep_alive(app, skb); break; - case NFP_FLOWER_CMSG_TYPE_TUN_NEIGH: - /* Acks from the NFP that the route is added - ignore. */ - break; default: nfp_flower_cmsg_warn(app, "Cannot handle invalid repr control type %u\n", type); @@ -306,6 +303,9 @@ void nfp_flower_cmsg_rx(struct nfp_app *app, struct sk_buff *skb) nfp_flower_process_mtu_ack(app, skb)) { /* Handle MTU acks outside wq to prevent RTNL conflict. */ dev_consume_skb_any(skb); + } else if (cmsg_hdr->type == NFP_FLOWER_CMSG_TYPE_TUN_NEIGH) { + /* Acks from the NFP that the route is added - ignore. */ + dev_consume_skb_any(skb); } else { skb_queue_tail(&priv->cmsg_skbs, skb); schedule_work(&priv->cmsg_work); From cf2cbadc20f5651c3dde9f5ac2ee52fb43aa4ddd Mon Sep 17 00:00:00 2001 From: Pieter Jansen van Vuuren Date: Wed, 11 Apr 2018 16:47:38 -0700 Subject: [PATCH 4/4] nfp: flower: split and limit cmsg skb lists Introduce a second skb list for handling control messages and limit the number of allowed messages. Some control messages are considered more crucial than others, resulting in the need for a second skb list. By splitting the list into a separate high and low priority list we can ensure that messages on the high list get added to the head of the list that gets processed, this however has no functional impact. Previously there was no limit on the number of messages allowed on the queue, this could result in the queue growing boundlessly and eventually the host running out of memory. Fixes: b985f870a5f0 ("nfp: process control messages in workqueue in flower app") Signed-off-by: Pieter Jansen van Vuuren Reviewed-by: Jakub Kicinski Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- .../net/ethernet/netronome/nfp/flower/cmsg.c | 38 +++++++++++++++++-- .../net/ethernet/netronome/nfp/flower/cmsg.h | 2 + .../net/ethernet/netronome/nfp/flower/main.c | 6 ++- .../net/ethernet/netronome/nfp/flower/main.h | 8 +++- 4 files changed, 46 insertions(+), 8 deletions(-) diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c index 8ad857eb89c6..577659f332e4 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c +++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c @@ -272,18 +272,49 @@ nfp_flower_cmsg_process_one_rx(struct nfp_app *app, struct sk_buff *skb) void nfp_flower_cmsg_process_rx(struct work_struct *work) { + struct sk_buff_head cmsg_joined; struct nfp_flower_priv *priv; struct sk_buff *skb; priv = container_of(work, struct nfp_flower_priv, cmsg_work); + skb_queue_head_init(&cmsg_joined); - while ((skb = skb_dequeue(&priv->cmsg_skbs))) + spin_lock_bh(&priv->cmsg_skbs_high.lock); + skb_queue_splice_tail_init(&priv->cmsg_skbs_high, &cmsg_joined); + spin_unlock_bh(&priv->cmsg_skbs_high.lock); + + spin_lock_bh(&priv->cmsg_skbs_low.lock); + skb_queue_splice_tail_init(&priv->cmsg_skbs_low, &cmsg_joined); + spin_unlock_bh(&priv->cmsg_skbs_low.lock); + + while ((skb = __skb_dequeue(&cmsg_joined))) nfp_flower_cmsg_process_one_rx(priv->app, skb); } +static void +nfp_flower_queue_ctl_msg(struct nfp_app *app, struct sk_buff *skb, int type) +{ + struct nfp_flower_priv *priv = app->priv; + struct sk_buff_head *skb_head; + + if (type == NFP_FLOWER_CMSG_TYPE_PORT_REIFY || + type == NFP_FLOWER_CMSG_TYPE_PORT_MOD) + skb_head = &priv->cmsg_skbs_high; + else + skb_head = &priv->cmsg_skbs_low; + + if (skb_queue_len(skb_head) >= NFP_FLOWER_WORKQ_MAX_SKBS) { + nfp_flower_cmsg_warn(app, "Dropping queued control messages\n"); + dev_kfree_skb_any(skb); + return; + } + + skb_queue_tail(skb_head, skb); + schedule_work(&priv->cmsg_work); +} + void nfp_flower_cmsg_rx(struct nfp_app *app, struct sk_buff *skb) { - struct nfp_flower_priv *priv = app->priv; struct nfp_flower_cmsg_hdr *cmsg_hdr; cmsg_hdr = nfp_flower_cmsg_get_hdr(skb); @@ -307,7 +338,6 @@ void nfp_flower_cmsg_rx(struct nfp_app *app, struct sk_buff *skb) /* Acks from the NFP that the route is added - ignore. */ dev_consume_skb_any(skb); } else { - skb_queue_tail(&priv->cmsg_skbs, skb); - schedule_work(&priv->cmsg_work); + nfp_flower_queue_ctl_msg(app, skb, cmsg_hdr->type); } } diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h index 96bc0e33980c..b6c0fd053a50 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.h +++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.h @@ -108,6 +108,8 @@ #define NFP_FL_IPV4_TUNNEL_TYPE GENMASK(7, 4) #define NFP_FL_IPV4_PRE_TUN_INDEX GENMASK(2, 0) +#define NFP_FLOWER_WORKQ_MAX_SKBS 30000 + #define nfp_flower_cmsg_warn(app, fmt, args...) \ do { \ if (net_ratelimit()) \ diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.c b/drivers/net/ethernet/netronome/nfp/flower/main.c index 6357e0720f43..ad02592a82b7 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/main.c +++ b/drivers/net/ethernet/netronome/nfp/flower/main.c @@ -519,7 +519,8 @@ static int nfp_flower_init(struct nfp_app *app) app->priv = app_priv; app_priv->app = app; - skb_queue_head_init(&app_priv->cmsg_skbs); + skb_queue_head_init(&app_priv->cmsg_skbs_high); + skb_queue_head_init(&app_priv->cmsg_skbs_low); INIT_WORK(&app_priv->cmsg_work, nfp_flower_cmsg_process_rx); init_waitqueue_head(&app_priv->reify_wait_queue); @@ -549,7 +550,8 @@ static void nfp_flower_clean(struct nfp_app *app) { struct nfp_flower_priv *app_priv = app->priv; - skb_queue_purge(&app_priv->cmsg_skbs); + skb_queue_purge(&app_priv->cmsg_skbs_high); + skb_queue_purge(&app_priv->cmsg_skbs_low); flush_work(&app_priv->cmsg_work); nfp_flower_metadata_cleanup(app); diff --git a/drivers/net/ethernet/netronome/nfp/flower/main.h b/drivers/net/ethernet/netronome/nfp/flower/main.h index e030b3ce4510..c67e1b54c614 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/main.h +++ b/drivers/net/ethernet/netronome/nfp/flower/main.h @@ -107,7 +107,10 @@ struct nfp_mtu_conf { * @mask_table: Hash table used to store masks * @flow_table: Hash table used to store flower rules * @cmsg_work: Workqueue for control messages processing - * @cmsg_skbs: List of skbs for control message processing + * @cmsg_skbs_high: List of higher priority skbs for control message + * processing + * @cmsg_skbs_low: List of lower priority skbs for control message + * processing * @nfp_mac_off_list: List of MAC addresses to offload * @nfp_mac_index_list: List of unique 8-bit indexes for non NFP netdevs * @nfp_ipv4_off_list: List of IPv4 addresses to offload @@ -136,7 +139,8 @@ struct nfp_flower_priv { DECLARE_HASHTABLE(mask_table, NFP_FLOWER_MASK_HASH_BITS); DECLARE_HASHTABLE(flow_table, NFP_FLOWER_HASH_BITS); struct work_struct cmsg_work; - struct sk_buff_head cmsg_skbs; + struct sk_buff_head cmsg_skbs_high; + struct sk_buff_head cmsg_skbs_low; struct list_head nfp_mac_off_list; struct list_head nfp_mac_index_list; struct list_head nfp_ipv4_off_list;