| Message ID | 20180120015408.16539-1-jakub.kicinski@netronome.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next] nfp: flower: prioritize stats updates | expand |
From: Jakub Kicinski <jakub.kicinski@netronome.com> Date: Fri, 19 Jan 2018 17:54:08 -0800 > From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> > > Previously it was possible to interrupt processing stats updates because > they were handled in a work queue. Interrupting the stats updates could > lead to a situation where we backup the control message queue. This patch > moves the stats update processing out of the work queue to be processed as > soon as hardware sends a request. > > Reported-by: Louis Peens <louis.peens@netronome.com> > Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> > Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com> > Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> Applied, thanks Jakub. Just a reminder that it really makes things more difficult that one can't just go: make drivers/net/ethernet/netronome/nfp/flower/cmsg.o to test a specific NFP object build like one can with pretty much the rest of the entire kernel tree.
On Sun, Jan 21, 2018 at 3:10 PM, David Miller <davem@davemloft.net> wrote: > From: Jakub Kicinski <jakub.kicinski@netronome.com> > Date: Fri, 19 Jan 2018 17:54:08 -0800 > >> From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> >> >> Previously it was possible to interrupt processing stats updates because >> they were handled in a work queue. Interrupting the stats updates could >> lead to a situation where we backup the control message queue. This patch >> moves the stats update processing out of the work queue to be processed as >> soon as hardware sends a request. >> >> Reported-by: Louis Peens <louis.peens@netronome.com> >> Signed-off-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com> >> Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com> >> Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com> > > Applied, thanks Jakub. Thank you! > Just a reminder that it really makes things more difficult that one > can't just go: > > make drivers/net/ethernet/netronome/nfp/flower/cmsg.o > > to test a specific NFP object build like one can with pretty much the > rest of the entire kernel tree. Oh, thanks for letting me know, I wasn't aware the build system seems to require a separate Makefile in each directory.. I'll fix it first thing in the morning!
On Sun, 21 Jan 2018 23:53:05 -0800, Jakub Kicinski wrote: > > Just a reminder that it really makes things more difficult that one > > can't just go: > > > > make drivers/net/ethernet/netronome/nfp/flower/cmsg.o > > > > to test a specific NFP object build like one can with pretty much the > > rest of the entire kernel tree. > > Oh, thanks for letting me know, I wasn't aware the build system seems > to require a separate Makefile in each directory.. I'll fix it first > thing in the morning! I failed to find a way to describe a multiple multi-dir module with Kbuild cleanly. I asked on linux-kbuild: https://www.spinics.net/lists/linux-kbuild/msg16509.html and unless someone speaks up I'll just create empty Makefiles :(
diff --git a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c index 615314d9e7c6..baaea6f1a9d8 100644 --- a/drivers/net/ethernet/netronome/nfp/flower/cmsg.c +++ b/drivers/net/ethernet/netronome/nfp/flower/cmsg.c @@ -211,12 +211,6 @@ nfp_flower_cmsg_process_one_rx(struct nfp_app *app, struct sk_buff *skb) cmsg_hdr = nfp_flower_cmsg_get_hdr(skb); - if (unlikely(cmsg_hdr->version != NFP_FLOWER_CMSG_VER1)) { - nfp_flower_cmsg_warn(app, "Cannot handle repr control version %u\n", - cmsg_hdr->version); - goto out; - } - type = cmsg_hdr->type; switch (type) { case NFP_FLOWER_CMSG_TYPE_PORT_REIFY: @@ -225,9 +219,6 @@ nfp_flower_cmsg_process_one_rx(struct nfp_app *app, struct sk_buff *skb) case NFP_FLOWER_CMSG_TYPE_PORT_MOD: nfp_flower_cmsg_portmod_rx(app, skb); break; - case NFP_FLOWER_CMSG_TYPE_FLOW_STATS: - nfp_flower_rx_flow_stats(app, skb); - break; case NFP_FLOWER_CMSG_TYPE_NO_NEIGH: nfp_tunnel_request_route(app, skb); break; @@ -263,7 +254,23 @@ void nfp_flower_cmsg_process_rx(struct work_struct *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); + + if (unlikely(cmsg_hdr->version != NFP_FLOWER_CMSG_VER1)) { + nfp_flower_cmsg_warn(app, "Cannot handle repr control version %u\n", + cmsg_hdr->version); + dev_kfree_skb_any(skb); + return; + } - skb_queue_tail(&priv->cmsg_skbs, skb); - schedule_work(&priv->cmsg_work); + if (cmsg_hdr->type == NFP_FLOWER_CMSG_TYPE_FLOW_STATS) { + /* We need to deal with stats updates from HW asap */ + nfp_flower_rx_flow_stats(app, skb); + dev_consume_skb_any(skb); + } else { + skb_queue_tail(&priv->cmsg_skbs, skb); + schedule_work(&priv->cmsg_work); + } }