netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code
@ 2019-08-28  5:56 Jakub Kicinski
  2019-08-28  5:56 ` [PATCH net 1/2] nfp: flower: prevent ingress block binds on internal ports Jakub Kicinski
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jakub Kicinski @ 2019-08-28  5:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski

John says:

There are few bugs in the merge encap code that have come to light with
recent driver changes. Effectively, flow bind callbacks were being
registered twice when using internal ports (new 'busy' code triggers
this). There was also an issue with neighbour notifier messages being
ignored for internal ports.

John Hurley (2):
  nfp: flower: prevent ingress block binds on internal ports
  nfp: flower: handle neighbour events on internal ports

 drivers/net/ethernet/netronome/nfp/flower/offload.c     | 7 ++++---
 drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c | 8 ++++----
 2 files changed, 8 insertions(+), 7 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net 1/2] nfp: flower: prevent ingress block binds on internal ports
  2019-08-28  5:56 [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code Jakub Kicinski
@ 2019-08-28  5:56 ` Jakub Kicinski
  2019-08-28  5:56 ` [PATCH net 2/2] nfp: flower: handle neighbour events " Jakub Kicinski
  2019-08-28 23:08 ` [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2019-08-28  5:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, John Hurley, Jakub Kicinski

From: John Hurley <john.hurley@netronome.com>

Internal port TC offload is implemented through user-space applications
(such as OvS) by adding filters at egress via TC clsact qdiscs. Indirect
block offload support in the NFP driver accepts both ingress qdisc binds
and egress binds if the device is an internal port. However, clsact sends
bind notification for both ingress and egress block binds which can lead
to the driver registering multiple callbacks and receiving multiple
notifications of new filters.

Fix this by rejecting ingress block bind callbacks when the port is
internal and only adding filter callbacks for egress binds.

Fixes: 4d12ba42787b ("nfp: flower: allow offloading of matches on 'internal' ports")
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/offload.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
index 9917d64694c6..457bdc60f3ee 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
@@ -1409,9 +1409,10 @@ nfp_flower_setup_indr_tc_block(struct net_device *netdev, struct nfp_app *app,
 	struct nfp_flower_priv *priv = app->priv;
 	struct flow_block_cb *block_cb;
 
-	if (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS &&
-	    !(f->binder_type == FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS &&
-	      nfp_flower_internal_port_can_offload(app, netdev)))
+	if ((f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS &&
+	     !nfp_flower_internal_port_can_offload(app, netdev)) ||
+	    (f->binder_type != FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS &&
+	     nfp_flower_internal_port_can_offload(app, netdev)))
 		return -EOPNOTSUPP;
 
 	switch (f->command) {
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH net 2/2] nfp: flower: handle neighbour events on internal ports
  2019-08-28  5:56 [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code Jakub Kicinski
  2019-08-28  5:56 ` [PATCH net 1/2] nfp: flower: prevent ingress block binds on internal ports Jakub Kicinski
@ 2019-08-28  5:56 ` Jakub Kicinski
  2019-08-28 23:08 ` [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2019-08-28  5:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, John Hurley, Simon Horman, Jakub Kicinski

From: John Hurley <john.hurley@netronome.com>

Recent code changes to NFP allowed the offload of neighbour entries to FW
when the next hop device was an internal port. This allows for offload of
tunnel encap when the end-point IP address is applied to such a port.

Unfortunately, the neighbour event handler still rejects events that are
not associated with a repr dev and so the firmware neighbour table may get
out of sync for internal ports.

Fix this by allowing internal port neighbour events to be correctly
processed.

Fixes: 45756dfedab5 ("nfp: flower: allow tunnels to output to internal port")
Signed-off-by: John Hurley <john.hurley@netronome.com>
Reviewed-by: Simon Horman <simon.horman@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
index a7a80f4b722a..f0ee982eb1b5 100644
--- a/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
+++ b/drivers/net/ethernet/netronome/nfp/flower/tunnel_conf.c
@@ -328,13 +328,13 @@ nfp_tun_neigh_event_handler(struct notifier_block *nb, unsigned long event,
 
 	flow.daddr = *(__be32 *)n->primary_key;
 
-	/* Only concerned with route changes for representors. */
-	if (!nfp_netdev_is_nfp_repr(n->dev))
-		return NOTIFY_DONE;
-
 	app_priv = container_of(nb, struct nfp_flower_priv, tun.neigh_nb);
 	app = app_priv->app;
 
+	if (!nfp_netdev_is_nfp_repr(n->dev) &&
+	    !nfp_flower_internal_port_can_offload(app, n->dev))
+		return NOTIFY_DONE;
+
 	/* Only concerned with changes to routes already added to NFP. */
 	if (!nfp_tun_has_route(app, flow.daddr))
 		return NOTIFY_DONE;
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code
  2019-08-28  5:56 [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code Jakub Kicinski
  2019-08-28  5:56 ` [PATCH net 1/2] nfp: flower: prevent ingress block binds on internal ports Jakub Kicinski
  2019-08-28  5:56 ` [PATCH net 2/2] nfp: flower: handle neighbour events " Jakub Kicinski
@ 2019-08-28 23:08 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2019-08-28 23:08 UTC (permalink / raw)
  To: jakub.kicinski; +Cc: netdev, oss-drivers

From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 27 Aug 2019 22:56:28 -0700

> John says:
> 
> There are few bugs in the merge encap code that have come to light with
> recent driver changes. Effectively, flow bind callbacks were being
> registered twice when using internal ports (new 'busy' code triggers
> this). There was also an issue with neighbour notifier messages being
> ignored for internal ports.

Series applied and queued up for v5.2 -stable, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-28 23:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28  5:56 [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code Jakub Kicinski
2019-08-28  5:56 ` [PATCH net 1/2] nfp: flower: prevent ingress block binds on internal ports Jakub Kicinski
2019-08-28  5:56 ` [PATCH net 2/2] nfp: flower: handle neighbour events " Jakub Kicinski
2019-08-28 23:08 ` [PATCH net 0/2] nfp: flower: fix bugs in merge tunnel encap code David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).