update xdp_features flag according to NIC re-configuration

PATCH Applied Lorenzo Bianconi 7 participants 24 messages Mar 9, 2023 – Apr 6, 2023

Subsystems (10)

Networking Network drivers BPF Mellanox mlx5 core +6 more subsystems

lore

Share & tools

lore

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #0 lore raw

v2 cover update xdp_features flag according to NIC re-configuration

Changes since v1:
- rebase on top of net tree
- remove NETDEV_XDP_ACT_NDO_XMIT_SG support in mlx5e driver
- always enable NETDEV_XDP_ACT_NDO_XMIT support in mlx5e driver

Lorenzo Bianconi (7):
tools: ynl: fix render-max for flags definition
tools: ynl: fix get_mask utility routine
xdp: add xdp_set_features_flag utility routine
net: thunderx: take into account xdp_features setting tx/rx queues
net: ena: take into account xdp_features setting tx/rx queues
veth: take into account device reconfiguration for xdp_features flag net/mlx5e: take into account device reconfiguration for xdp_features
flag

Matteo Croce (1):
mvpp2: take care of xdp_features when reconfiguring queues

 Documentation/netlink/specs/netdev.yaml       |  1 +
 drivers/net/ethernet/amazon/ena/ena_ethtool.c | 15 +++++--
 drivers/net/ethernet/amazon/ena/ena_netdev.c  |  6 ++-
 .../ethernet/cavium/thunder/nicvf_ethtool.c   | 17 +++++---
 .../net/ethernet/cavium/thunder/nicvf_main.c  |  4 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   | 15 +++++--
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  1 +
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  | 10 ++++-
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 37 +++++++++++-----
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |  3 ++
 drivers/net/veth.c                            | 42 +++++++++++++++++--
 include/net/xdp.h                             | 11 +++++
 include/uapi/linux/netdev.h                   |  2 +
 net/core/xdp.c                                | 26 ++++++++----
 tools/include/uapi/linux/netdev.h             |  2 +
 tools/net/ynl/lib/nlspec.py                   |  6 +--
 tools/net/ynl/ynl-gen-c.py                    | 11 +++--
 17 files changed, 164 insertions(+), 45 deletions(-)

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #1 lore raw

PATCH v2 1/8 tools: ynl: fix render-max for flags definition

Properly manage render-max property for flags definition type introducing mask value and setting it to (last_element << 1) - 1 instead of adding max value set to last_element + 1

Fixes be5bea1cc0bf ("net: add basic C code generators for Netlink")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 tools/net/ynl/ynl-gen-c.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

tools/net/ynl/ynl-gen-c.py +8 -3

index 1bcc5354d800..d47376f19de7 100755
@@ -1931,9 +1931,14 @@ def render_uapi(family, cw):
 
             if const.get('render-max', False):
                 cw.nl()
-                max_name = c_upper(name_pfx + 'max')
-                cw.p('__' + max_name + ',')
-                cw.p(max_name + ' = (__' + max_name + ' - 1)')
+                if const['type'] == 'flags':
+                    max_name = c_upper(name_pfx + 'mask')
+                    max_val = f' = {enum.get_mask()},'
+                    cw.p(max_name + max_val)
+                else:
+                    max_name = c_upper(name_pfx + 'max')
+                    cw.p('__' + max_name + ',')
+                    cw.p(max_name + ' = (__' + max_name + ' - 1)')
             cw.block_end(line=';')
             cw.nl()
         elif const['type'] == 'const':

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #2 lore raw

PATCH v2 2/8 tools: ynl: fix get_mask utility routine

In reply to Lorenzo Bianconi

Fix get_mask utility routine in order to take into account possible gaps in the elements list.

Fixes be5bea1cc0bf ("net: add basic C code generators for Netlink")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 tools/net/ynl/lib/nlspec.py | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

tools/net/ynl/lib/nlspec.py +2 -4

index a34d088f6743..960a356e8225 100644
@@ -138,10 +138,8 @@ class SpecEnumSet(SpecElement):
 
     def get_mask(self):
         mask = 0
-        idx = self.yaml.get('value-start', 0)
-        for _ in self.entries.values():
-            mask |= 1 << idx
-            idx += 1
+        for e in self.entries.values():
+            mask += e.user_value()
         return mask

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #3 lore raw

PATCH v2 3/8 xdp: add xdp_set_features_flag utility routine

In reply to Lorenzo Bianconi

Introduce xdp_set_features_flag utility routine in order to update dynamically xdp_features according to the dynamic hw configuration via ethtool (e.g. changing number of hw rx/tx queues).
Add xdp_clear_features_flag() in order to clear all xdp_feature flag.

Reviewed-by Shay Agroskin <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 Documentation/netlink/specs/netdev.yaml |  1 +
 include/net/xdp.h                       | 11 +++++++++++
 include/uapi/linux/netdev.h             |  2 ++
 net/core/xdp.c                          | 26 ++++++++++++++++++-------
 tools/include/uapi/linux/netdev.h       |  2 ++
 5 files changed, 35 insertions(+), 7 deletions(-)

Documentation/netlink/specs/netdev.yaml +1 -0

index 24de747b5344..753e5914a8b7 100644
@@ -9,6 +9,7 @@ definitions:
   -
     type: flags
     name: xdp-act
+    render-max: true
     entries:
       -
         name: basic

include/net/xdp.h +11 -0

index d517bfac937b..41c57b8b1671 100644
@@ -428,12 +428,18 @@ MAX_XDP_METADATA_KFUNC,
 #ifdef CONFIG_NET
 u32 bpf_xdp_metadata_kfunc_id(int id);
 bool bpf_dev_bound_kfunc_id(u32 btf_id);
+void xdp_set_features_flag(struct net_device *dev, xdp_features_t val);
 void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg);
 void xdp_features_clear_redirect_target(struct net_device *dev);
 #else
 static inline u32 bpf_xdp_metadata_kfunc_id(int id) { return 0; }
 static inline bool bpf_dev_bound_kfunc_id(u32 btf_id) { return false; }
 
+static inline void
+xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
+{
+}
+
 static inline void
 xdp_features_set_redirect_target(struct net_device *dev, bool support_sg)
 {

@@ -445,4 +451,9 @@ xdp_features_clear_redirect_target(struct net_device *dev)
 }
 #endif
 
+static inline void xdp_clear_features_flag(struct net_device *dev)
+{
+	xdp_set_features_flag(dev, 0);
+}
+
 #endif /* __LINUX_NET_XDP_H__ */

include/uapi/linux/netdev.h +2 -0

index 8c4e3e536c04..ed134fbdfd32 100644
@@ -33,6 +33,8 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
 	NETDEV_XDP_ACT_RX_SG = 32,
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+
+	NETDEV_XDP_ACT_MASK = 127,
 };
 
 enum {

net/core/xdp.c +19 -7

index 8c92fc553317..87e654b7d06c 100644
@@ -774,20 +774,32 @@ static int __init xdp_metadata_init(void)
 }
 late_initcall(xdp_metadata_init);
 
-void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg)
+void xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
 {
-	dev->xdp_features |= NETDEV_XDP_ACT_NDO_XMIT;
-	if (support_sg)
-		dev->xdp_features |= NETDEV_XDP_ACT_NDO_XMIT_SG;
+	val &= NETDEV_XDP_ACT_MASK;
+	if (dev->xdp_features == val)
+		return;
 
+	dev->xdp_features = val;
 	call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev);
 }
+EXPORT_SYMBOL_GPL(xdp_set_features_flag);
+
+void xdp_features_set_redirect_target(struct net_device *dev, bool support_sg)
+{
+	xdp_features_t val = (dev->xdp_features | NETDEV_XDP_ACT_NDO_XMIT);
+
+	if (support_sg)
+		val |= NETDEV_XDP_ACT_NDO_XMIT_SG;
+	xdp_set_features_flag(dev, val);
+}
 EXPORT_SYMBOL_GPL(xdp_features_set_redirect_target);
 
 void xdp_features_clear_redirect_target(struct net_device *dev)
 {
-	dev->xdp_features &= ~(NETDEV_XDP_ACT_NDO_XMIT |
-			       NETDEV_XDP_ACT_NDO_XMIT_SG);
-	call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev);
+	xdp_features_t val = dev->xdp_features;
+
+	val &= ~(NETDEV_XDP_ACT_NDO_XMIT | NETDEV_XDP_ACT_NDO_XMIT_SG);
+	xdp_set_features_flag(dev, val);
 }
 EXPORT_SYMBOL_GPL(xdp_features_clear_redirect_target);

tools/include/uapi/linux/netdev.h +2 -0

index 8c4e3e536c04..ed134fbdfd32 100644
@@ -33,6 +33,8 @@ enum netdev_xdp_act {
 	NETDEV_XDP_ACT_HW_OFFLOAD = 16,
 	NETDEV_XDP_ACT_RX_SG = 32,
 	NETDEV_XDP_ACT_NDO_XMIT_SG = 64,
+
+	NETDEV_XDP_ACT_MASK = 127,
 };
 
 enum {

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #4 lore raw

PATCH v2 4/8 net: thunderx: take into account xdp_features setting tx/rx queues

In reply to Lorenzo Bianconi

thunderx nic allows xdp just if enough hw queues are available for XDP. Take into account queues configuration setting xdp_features.

Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 .../net/ethernet/cavium/thunder/nicvf_ethtool.c | 17 +++++++++++------
 .../net/ethernet/cavium/thunder/nicvf_main.c    |  4 +++-
 2 files changed, 14 insertions(+), 7 deletions(-)

drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c +11 -6

index e5c71f907852..d8d71bf97983 100644
@@ -735,12 +735,17 @@ static int nicvf_set_channels(struct net_device *dev,
 	if (channel->tx_count > nic->max_queues)
 		return -EINVAL;
 
-	if (nic->xdp_prog &&
-	    ((channel->tx_count + channel->rx_count) > nic->max_queues)) {
-		netdev_err(nic->netdev,
-			   "XDP mode, RXQs + TXQs > Max %d\n",
-			   nic->max_queues);
-		return -EINVAL;
+	if (channel->tx_count + channel->rx_count > nic->max_queues) {
+		if (nic->xdp_prog) {
+			netdev_err(nic->netdev,
+				   "XDP mode, RXQs + TXQs > Max %d\n",
+				   nic->max_queues);
+			return -EINVAL;
+		}
+
+		xdp_clear_features_flag(nic->netdev);
+	} else if (!pass1_silicon(nic->pdev)) {
+		xdp_set_features_flag(dev, NETDEV_XDP_ACT_BASIC);
 	}
 
 	if (if_up)

drivers/net/ethernet/cavium/thunder/nicvf_main.c +3 -1

index 8b25313c7f6b..eff350e0bc2a 100644
@@ -2218,7 +2218,9 @@ static int nicvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	netdev->netdev_ops = &nicvf_netdev_ops;
 	netdev->watchdog_timeo = NICVF_TX_TIMEOUT;
 
-	netdev->xdp_features = NETDEV_XDP_ACT_BASIC;
+	if (!pass1_silicon(nic->pdev) &&
+	    nic->rx_queues + nic->tx_queues <= nic->max_queues)
+		netdev->xdp_features = NETDEV_XDP_ACT_BASIC;
 
 	/* MTU range: 64 - 9200 */
 	netdev->min_mtu = NIC_HW_MIN_FRS;

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #5 lore raw

PATCH v2 5/8 net: ena: take into account xdp_features setting tx/rx queues

In reply to Lorenzo Bianconi

ena nic allows xdp just if enough hw queues are available for XDP. Take into account queues configuration setting xdp_features.

Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")

Reviewed-by Shay Agroskin <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/amazon/ena/ena_ethtool.c | 15 ++++++++++++---
 drivers/net/ethernet/amazon/ena/ena_netdev.c  |  6 ++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

drivers/net/ethernet/amazon/ena/ena_ethtool.c +12 -3

index 8da79eedc057..1d4f2f4d10f2 100644
@@ -850,11 +850,20 @@ static int ena_set_channels(struct net_device *netdev,
 	struct ena_adapter *adapter = netdev_priv(netdev);
 	u32 count = channels->combined_count;
 	/* The check for max value is already done in ethtool */
-	if (count < ENA_MIN_NUM_IO_QUEUES ||
-	    (ena_xdp_present(adapter) &&
-	    !ena_xdp_legal_queue_count(adapter, count)))
+	if (count < ENA_MIN_NUM_IO_QUEUES)
 		return -EINVAL;
 
+	if (!ena_xdp_legal_queue_count(adapter, count)) {
+		if (ena_xdp_present(adapter))
+			return -EINVAL;
+
+		xdp_clear_features_flag(netdev);
+	} else {
+		xdp_set_features_flag(netdev,
+				      NETDEV_XDP_ACT_BASIC |
+				      NETDEV_XDP_ACT_REDIRECT);
+	}
+
 	return ena_update_queue_count(adapter, count);
 }

drivers/net/ethernet/amazon/ena/ena_netdev.c +4 -2

index d3999db7c6a2..cbfe7f977270 100644
@@ -4105,8 +4105,6 @@ static void ena_set_conf_feat_params(struct ena_adapter *adapter,
 	/* Set offload features */
 	ena_set_dev_offloads(feat, netdev);
 
-	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT;
-
 	adapter->max_mtu = feat->dev_attr.max_mtu;
 	netdev->max_mtu = adapter->max_mtu;
 	netdev->min_mtu = ENA_MIN_MTU;

@@ -4393,6 +4391,10 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	ena_config_debug_area(adapter);
 
+	if (ena_xdp_legal_queue_count(adapter, adapter->num_io_queues))
+		netdev->xdp_features = NETDEV_XDP_ACT_BASIC |
+				       NETDEV_XDP_ACT_REDIRECT;
+
 	memcpy(adapter->netdev->perm_addr, adapter->mac_addr, netdev->addr_len);
 
 	netif_carrier_off(netdev);

Signature · 1 line

2.39.2

Lorenzo Bianconi

author

2023-03-09 12:26 UTC #6 lore raw

PATCH v2 6/8 veth: take into account device reconfiguration for xdp_features flag

In reply to Lorenzo Bianconi

Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.

Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/veth.c | 42 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 4 deletions(-)

drivers/net/veth.c +38 -4

index 1bb54de7124d..293dc3b2c84a 100644
@@ -1257,6 +1257,26 @@ static int veth_enable_range_safe(struct net_device *dev, int start, int end)
 	return 0;
 }
 
+static void veth_set_xdp_features(struct net_device *dev)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;
+
+	peer = rcu_dereference(priv->peer);
+	if (peer && peer->real_num_tx_queues <= dev->real_num_rx_queues) {
+		xdp_features_t val = NETDEV_XDP_ACT_BASIC |
+				     NETDEV_XDP_ACT_REDIRECT |
+				     NETDEV_XDP_ACT_RX_SG;
+
+		if (priv->_xdp_prog || veth_gro_requested(dev))
+			val |= NETDEV_XDP_ACT_NDO_XMIT |
+			       NETDEV_XDP_ACT_NDO_XMIT_SG;
+		xdp_set_features_flag(dev, val);
+	} else {
+		xdp_clear_features_flag(dev);
+	}
+}
+
 static int veth_set_channels(struct net_device *dev,
 			     struct ethtool_channels *ch)
 {

@@ -1323,6 +1343,12 @@ static int veth_set_channels(struct net_device *dev,
 		if (peer)
 			netif_carrier_on(peer);
 	}
+
+	/* update XDP supported features */
+	veth_set_xdp_features(dev);
+	if (peer)
+		veth_set_xdp_features(peer);
+
 	return err;
 
 revert:

@@ -1489,7 +1515,10 @@ static int veth_set_features(struct net_device *dev,
 		err = veth_napi_enable(dev);
 		if (err)
 			return err;
+
+		xdp_features_set_redirect_target(dev, true);
 	} else {
+		xdp_features_clear_redirect_target(dev);
 		veth_napi_del(dev);
 	}
 	return 0;

@@ -1570,10 +1599,15 @@ static int veth_xdp_set(struct net_device *dev, struct bpf_prog *prog,
 			peer->hw_features &= ~NETIF_F_GSO_SOFTWARE;
 			peer->max_mtu = max_mtu;
 		}
+
+		xdp_features_set_redirect_target(dev, true);
 	}
 
 	if (old_prog) {
 		if (!prog) {
+			if (!veth_gro_requested(dev))
+				xdp_features_clear_redirect_target(dev);
+
 			if (dev->flags & IFF_UP)
 				veth_disable_xdp(dev);

@@ -1686,10 +1720,6 @@ static void veth_setup(struct net_device *dev)
 	dev->hw_enc_features = VETH_FEATURES;
 	dev->mpls_features = NETIF_F_HW_CSUM | NETIF_F_GSO_SOFTWARE;
 	netif_set_tso_max_size(dev, GSO_MAX_SIZE);
-
-	dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-			    NETDEV_XDP_ACT_NDO_XMIT | NETDEV_XDP_ACT_RX_SG |
-			    NETDEV_XDP_ACT_NDO_XMIT_SG;
 }
 
 /*

@@ -1857,6 +1887,10 @@ static int veth_newlink(struct net *src_net, struct net_device *dev,
 		goto err_queues;
 
 	veth_disable_gro(dev);
+	/* update XDP supported features */
+	veth_set_xdp_features(dev);
+	veth_set_xdp_features(peer);
+
 	return 0;
 
 err_queues:

Signature · 1 line

2.39.2

Matthieu Baerts

2023-03-13 14:15 UTC #7 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Hi Lorenzo,

Quoted 6 lines · Lorenzo Bianconi

Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>

Thank you for the modification.

Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:

###################### 8< ######################

=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!

other info that might help us debug this:

rcu_scheduler_active = 2, debug_locks = 1
1 lock held by ip/135:
#0: ffffffff8dc4b108 (rtnl_mutex){+.+.}-{3:3}, at: rtnetlink_rcv_msg (net/core/rtnetlink.c:6172)

stack backtrace:
CPU: 1 PID: 135 Comm: ip Not tainted 6.3.0-rc1-00144-g064d70527aaa #149
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
04/01/2014
Call Trace:
 <TASK>

dump_stack_lvl (lib/dump_stack.c:107) lockdep_rcu_suspicious (include/linux/context_tracking.h:152) veth_set_xdp_features (drivers/net/veth.c:1265 (discriminator 9)) veth_newlink (drivers/net/veth.c:1892)
? veth_set_features (drivers/net/veth.c:1774)
? kasan_save_stack (mm/kasan/common.c:47)
? kasan_save_stack (mm/kasan/common.c:46)
? kasan_set_track (mm/kasan/common.c:52)
? alloc_netdev_mqs (include/linux/slab.h:737)
? rcu_read_lock_sched_held (kernel/rcu/update.c:125)
? trace_kmalloc (include/trace/events/kmem.h:54)
? __xdp_rxq_info_reg (net/core/xdp.c:188)
? alloc_netdev_mqs (net/core/dev.c:10657)
? rtnl_create_link (net/core/rtnetlink.c:3312) rtnl_newlink_create (net/core/rtnetlink.c:3440)
? rtnl_link_get_net_capable.constprop.0 (net/core/rtnetlink.c:3391) __rtnl_newlink (net/core/rtnetlink.c:3657)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? rtnl_link_unregister (net/core/rtnetlink.c:3487) rtnl_newlink (net/core/rtnetlink.c:3671) rtnetlink_rcv_msg (net/core/rtnetlink.c:6174)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? mark_usage (kernel/locking/lockdep.c:4914)
? mark_usage (kernel/locking/lockdep.c:4914) netlink_rcv_skb (net/netlink/af_netlink.c:2574)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? netlink_ack (net/netlink/af_netlink.c:2551)
? lock_acquire (kernel/locking/lockdep.c:467)
? net_generic (include/linux/rcupdate.h:805)
? netlink_deliver_tap (include/linux/rcupdate.h:805) netlink_unicast (net/netlink/af_netlink.c:1340)
? netlink_attachskb (net/netlink/af_netlink.c:1350) netlink_sendmsg (net/netlink/af_netlink.c:1942)
? netlink_unicast (net/netlink/af_netlink.c:1861)
? netlink_unicast (net/netlink/af_netlink.c:1861) sock_sendmsg (net/socket.c:727)
____sys_sendmsg (net/socket.c:2501)
? kernel_sendmsg (net/socket.c:2448)
? __copy_msghdr (net/socket.c:2428)
___sys_sendmsg (net/socket.c:2557)
? mark_usage (kernel/locking/lockdep.c:4914)
? do_recvmmsg (net/socket.c:2544)
? lock_acquire (kernel/locking/lockdep.c:467)
? find_held_lock (kernel/locking/lockdep.c:5159)
? __lock_release (kernel/locking/lockdep.c:5345)
? __might_fault (mm/memory.c:5625)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? __fget_light (include/linux/atomic/atomic-arch-fallback.h:227) __sys_sendmsg (include/linux/file.h:31)
? __sys_sendmsg_sock (net/socket.c:2572)
? rseq_get_rseq_cs (kernel/rseq.c:275)
? lockdep_hardirqs_on_prepare.part.0 (kernel/locking/lockdep.c:4263) do_syscall_64 (arch/x86/entry/common.c:50) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)

RIP: 0033:0x7f0d1aadeb17
Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e

fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10 All code
========

   0:   0f 00                   (bad)
   2:   f7 d8                   neg    %eax
   4:   64 89 02                mov    %eax,%fs:(%rdx)
   7:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
   e:   eb b9                   jmp    0xffffffffffffffc9
  10:   0f 1f 00                nopl   (%rax)
  13:   f3 0f 1e fa             endbr64
  17:   64 8b 04 25 18 00 00    mov    %fs:0x18,%eax
  1e:   00
  1f:   85 c0                   test   %eax,%eax
  21:   75 10                   jne    0x33
  23:   b8 2e 00 00 00          mov    $0x2e,%eax
  28:   0f 05                   syscall
  2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
<-- trapping instruction
  30:   77 51                   ja     0x83
  32:   c3                      ret
  33:   48 83 ec 28             sub    $0x28,%rsp
  37:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  3b:   48 89 74 24 10          mov    %rsi,0x10(%rsp)

Code starting with the faulting instruction
===========================================

   0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
   6:   77 51                   ja     0x59
   8:   c3                      ret
   9:   48 83 ec 28             sub    $0x28,%rsp
   d:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  11:   48 89 74 24 10          mov    %rsi,0x10(%rsp)

RSP: 002b:00007ffca3305d48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000640f2bb2 RCX: 00007f0d1aadeb17
RDX: 0000000000000000 RSI: 00007ffca3305db0 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffca3304ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffca3305eb4 R14: 00007ffca3305e80 R15: 0000561e28bf5020

 </TASK>

ip (135) used greatest stack depth: 24544 bytes left

###################### 8< ######################

I can reproduce the issue on the "net" tree with just these 3 commands:

# ip netns add ns1
# ip netns add ns2
# ip link add ns1eth1 netns ns1 type veth peer name ns2eth1 netns ns2

Without this commit fccca038f300 ("veth: take into account device reconfiguration for xdp_features flag"), I don't have the issue.

For more details about the issue detected by CIs validating our MPTCP tree, including kconfig and vmlinux if needed:

https://github.com/multipath-tcp/mptcp_net-next/issues/372

Do you mind looking at this regression please? :)

On our side, we will revert this patch in our tree for the moment to unblock our CI jobs.

Cheers,
Matt

Signature · 2 lines

Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Eric Dumazet

2023-03-13 15:51 UTC #8 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Quoted 19 lines · Matthieu Baerts

Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################

Quoted 8 lines

=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!
other info that might help us debug this:

Same observation here, I am releasing a syzbot report with a repro.

Quoted 139 lines

rcu_scheduler_active = 2, debug_locks = 1
1 lock held by ip/135:
#0: ffffffff8dc4b108 (rtnl_mutex){+.+.}-{3:3}, at: rtnetlink_rcv_msg (net/core/rtnetlink.c:6172)
stack backtrace:
CPU: 1 PID: 135 Comm: ip Not tainted 6.3.0-rc1-00144-g064d70527aaa #149
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (lib/dump_stack.c:107) lockdep_rcu_suspicious (include/linux/context_tracking.h:152) veth_set_xdp_features (drivers/net/veth.c:1265 (discriminator 9)) veth_newlink (drivers/net/veth.c:1892)
? veth_set_features (drivers/net/veth.c:1774)
? kasan_save_stack (mm/kasan/common.c:47)
? kasan_save_stack (mm/kasan/common.c:46)
? kasan_set_track (mm/kasan/common.c:52)
? alloc_netdev_mqs (include/linux/slab.h:737)
? rcu_read_lock_sched_held (kernel/rcu/update.c:125)
? trace_kmalloc (include/trace/events/kmem.h:54)
? __xdp_rxq_info_reg (net/core/xdp.c:188)
? alloc_netdev_mqs (net/core/dev.c:10657)
? rtnl_create_link (net/core/rtnetlink.c:3312) rtnl_newlink_create (net/core/rtnetlink.c:3440)
? rtnl_link_get_net_capable.constprop.0 (net/core/rtnetlink.c:3391) __rtnl_newlink (net/core/rtnetlink.c:3657)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? rtnl_link_unregister (net/core/rtnetlink.c:3487) rtnl_newlink (net/core/rtnetlink.c:3671) rtnetlink_rcv_msg (net/core/rtnetlink.c:6174)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? mark_usage (kernel/locking/lockdep.c:4914)
? mark_usage (kernel/locking/lockdep.c:4914) netlink_rcv_skb (net/netlink/af_netlink.c:2574)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? netlink_ack (net/netlink/af_netlink.c:2551)
? lock_acquire (kernel/locking/lockdep.c:467)
? net_generic (include/linux/rcupdate.h:805)
? netlink_deliver_tap (include/linux/rcupdate.h:805) netlink_unicast (net/netlink/af_netlink.c:1340)
? netlink_attachskb (net/netlink/af_netlink.c:1350) netlink_sendmsg (net/netlink/af_netlink.c:1942)
? netlink_unicast (net/netlink/af_netlink.c:1861)
? netlink_unicast (net/netlink/af_netlink.c:1861) sock_sendmsg (net/socket.c:727)
____sys_sendmsg (net/socket.c:2501)
? kernel_sendmsg (net/socket.c:2448)
? __copy_msghdr (net/socket.c:2428)
___sys_sendmsg (net/socket.c:2557)
? mark_usage (kernel/locking/lockdep.c:4914)
? do_recvmmsg (net/socket.c:2544)
? lock_acquire (kernel/locking/lockdep.c:467)
? find_held_lock (kernel/locking/lockdep.c:5159)
? __lock_release (kernel/locking/lockdep.c:5345)
? __might_fault (mm/memory.c:5625)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? __fget_light (include/linux/atomic/atomic-arch-fallback.h:227) __sys_sendmsg (include/linux/file.h:31)
? __sys_sendmsg_sock (net/socket.c:2572)
? rseq_get_rseq_cs (kernel/rseq.c:275)
? lockdep_hardirqs_on_prepare.part.0 (kernel/locking/lockdep.c:4263) do_syscall_64 (arch/x86/entry/common.c:50) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
RIP: 0033:0x7f0d1aadeb17
Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e
fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10 All code
========
   0:   0f 00                   (bad)
   2:   f7 d8                   neg    %eax
   4:   64 89 02                mov    %eax,%fs:(%rdx)
   7:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
   e:   eb b9                   jmp    0xffffffffffffffc9
  10:   0f 1f 00                nopl   (%rax)
  13:   f3 0f 1e fa             endbr64
  17:   64 8b 04 25 18 00 00    mov    %fs:0x18,%eax
  1e:   00
  1f:   85 c0                   test   %eax,%eax
  21:   75 10                   jne    0x33
  23:   b8 2e 00 00 00          mov    $0x2e,%eax
  28:   0f 05                   syscall
  2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
<-- trapping instruction
  30:   77 51                   ja     0x83
  32:   c3                      ret
  33:   48 83 ec 28             sub    $0x28,%rsp
  37:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  3b:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
Code starting with the faulting instruction
===========================================
   0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
   6:   77 51                   ja     0x59
   8:   c3                      ret
   9:   48 83 ec 28             sub    $0x28,%rsp
   d:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  11:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
RSP: 002b:00007ffca3305d48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000640f2bb2 RCX: 00007f0d1aadeb17
RDX: 0000000000000000 RSI: 00007ffca3305db0 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffca3304ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffca3305eb4 R14: 00007ffca3305e80 R15: 0000561e28bf5020
 </TASK>
ip (135) used greatest stack depth: 24544 bytes left
###################### 8< ######################
I can reproduce the issue on the "net" tree with just these 3 commands:
# ip netns add ns1
# ip netns add ns2
# ip link add ns1eth1 netns ns1 type veth peer name ns2eth1 netns ns2
Without this commit fccca038f300 ("veth: take into account device reconfiguration for xdp_features flag"), I don't have the issue.
For more details about the issue detected by CIs validating our MPTCP tree, including kconfig and vmlinux if needed:
https://github.com/multipath-tcp/mptcp_net-next/issues/372
Do you mind looking at this regression please? :)
On our side, we will revert this patch in our tree for the moment to unblock our CI jobs.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Eric Dumazet

2023-03-13 15:53 UTC #9 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Quoted 28 lines · Eric Dumazet

Quoted 25 lines · Matthieu Baerts
Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################
=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!

Quoted 6 lines

Quoted 2 lines
other info that might help us debug this:
Same observation here, I am releasing a syzbot report with a repro.

I guess a fix would be:

drivers/net/veth.c +0 -0

index 293dc3b2c84a6c1931e8df42cdcd5f2798004f3c..4da74ac27f9a2425d8d3f4ffcc93f453bd58e3a5

100644

drivers/net/veth.c +1 -1

@@ -1262,7 +1262,7 @@ static void veth_set_xdp_features(struct net_device *dev)
        struct veth_priv *priv = netdev_priv(dev);
        struct net_device *peer;
 
-       peer = rcu_dereference(priv->peer);
+       peer = rtnl_dereference(priv->peer);
        if (peer && peer->real_num_tx_queues <= dev->real_num_rx_queues) {
                xdp_features_t val = NETDEV_XDP_ACT_BASIC |
                                     NETDEV_XDP_ACT_REDIRECT |

Quoted 140 lines

Quoted 140 lines
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by ip/135:
#0: ffffffff8dc4b108 (rtnl_mutex){+.+.}-{3:3}, at: rtnetlink_rcv_msg (net/core/rtnetlink.c:6172)
stack backtrace:
CPU: 1 PID: 135 Comm: ip Not tainted 6.3.0-rc1-00144-g064d70527aaa #149
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (lib/dump_stack.c:107) lockdep_rcu_suspicious (include/linux/context_tracking.h:152) veth_set_xdp_features (drivers/net/veth.c:1265 (discriminator 9)) veth_newlink (drivers/net/veth.c:1892)
? veth_set_features (drivers/net/veth.c:1774)
? kasan_save_stack (mm/kasan/common.c:47)
? kasan_save_stack (mm/kasan/common.c:46)
? kasan_set_track (mm/kasan/common.c:52)
? alloc_netdev_mqs (include/linux/slab.h:737)
? rcu_read_lock_sched_held (kernel/rcu/update.c:125)
? trace_kmalloc (include/trace/events/kmem.h:54)
? __xdp_rxq_info_reg (net/core/xdp.c:188)
? alloc_netdev_mqs (net/core/dev.c:10657)
? rtnl_create_link (net/core/rtnetlink.c:3312) rtnl_newlink_create (net/core/rtnetlink.c:3440)
? rtnl_link_get_net_capable.constprop.0 (net/core/rtnetlink.c:3391) __rtnl_newlink (net/core/rtnetlink.c:3657)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? rtnl_link_unregister (net/core/rtnetlink.c:3487) rtnl_newlink (net/core/rtnetlink.c:3671) rtnetlink_rcv_msg (net/core/rtnetlink.c:6174)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? mark_usage (kernel/locking/lockdep.c:4914)
? mark_usage (kernel/locking/lockdep.c:4914) netlink_rcv_skb (net/netlink/af_netlink.c:2574)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? netlink_ack (net/netlink/af_netlink.c:2551)
? lock_acquire (kernel/locking/lockdep.c:467)
? net_generic (include/linux/rcupdate.h:805)
? netlink_deliver_tap (include/linux/rcupdate.h:805) netlink_unicast (net/netlink/af_netlink.c:1340)
? netlink_attachskb (net/netlink/af_netlink.c:1350) netlink_sendmsg (net/netlink/af_netlink.c:1942)
? netlink_unicast (net/netlink/af_netlink.c:1861)
? netlink_unicast (net/netlink/af_netlink.c:1861) sock_sendmsg (net/socket.c:727)
____sys_sendmsg (net/socket.c:2501)
? kernel_sendmsg (net/socket.c:2448)
? __copy_msghdr (net/socket.c:2428)
___sys_sendmsg (net/socket.c:2557)
? mark_usage (kernel/locking/lockdep.c:4914)
? do_recvmmsg (net/socket.c:2544)
? lock_acquire (kernel/locking/lockdep.c:467)
? find_held_lock (kernel/locking/lockdep.c:5159)
? __lock_release (kernel/locking/lockdep.c:5345)
? __might_fault (mm/memory.c:5625)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? __fget_light (include/linux/atomic/atomic-arch-fallback.h:227) __sys_sendmsg (include/linux/file.h:31)
? __sys_sendmsg_sock (net/socket.c:2572)
? rseq_get_rseq_cs (kernel/rseq.c:275)
? lockdep_hardirqs_on_prepare.part.0 (kernel/locking/lockdep.c:4263) do_syscall_64 (arch/x86/entry/common.c:50) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
RIP: 0033:0x7f0d1aadeb17
Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e
fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10 All code
========
   0:   0f 00                   (bad)
   2:   f7 d8                   neg    %eax
   4:   64 89 02                mov    %eax,%fs:(%rdx)
   7:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
   e:   eb b9                   jmp    0xffffffffffffffc9
  10:   0f 1f 00                nopl   (%rax)
  13:   f3 0f 1e fa             endbr64
  17:   64 8b 04 25 18 00 00    mov    %fs:0x18,%eax
  1e:   00
  1f:   85 c0                   test   %eax,%eax
  21:   75 10                   jne    0x33
  23:   b8 2e 00 00 00          mov    $0x2e,%eax
  28:   0f 05                   syscall
  2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
<-- trapping instruction
  30:   77 51                   ja     0x83
  32:   c3                      ret
  33:   48 83 ec 28             sub    $0x28,%rsp
  37:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  3b:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
Code starting with the faulting instruction
===========================================
   0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
   6:   77 51                   ja     0x59
   8:   c3                      ret
   9:   48 83 ec 28             sub    $0x28,%rsp
   d:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  11:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
RSP: 002b:00007ffca3305d48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000640f2bb2 RCX: 00007f0d1aadeb17
RDX: 0000000000000000 RSI: 00007ffca3305db0 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffca3304ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffca3305eb4 R14: 00007ffca3305e80 R15: 0000561e28bf5020
 </TASK>
ip (135) used greatest stack depth: 24544 bytes left
###################### 8< ######################
I can reproduce the issue on the "net" tree with just these 3 commands:
# ip netns add ns1
# ip netns add ns2
# ip link add ns1eth1 netns ns1 type veth peer name ns2eth1 netns ns2
Without this commit fccca038f300 ("veth: take into account device reconfiguration for xdp_features flag"), I don't have the issue.
For more details about the issue detected by CIs validating our MPTCP tree, including kconfig and vmlinux if needed:
https://github.com/multipath-tcp/mptcp_net-next/issues/372
Do you mind looking at this regression please? :)
On our side, we will revert this patch in our tree for the moment to unblock our CI jobs.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Matthieu Baerts

2023-03-13 16:32 UTC #10 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Hi Eric,

Quoted 45 lines · Eric Dumazet

Quoted 28 lines · Eric Dumazet
Quoted 25 lines · Matthieu Baerts
Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################
=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!
Quoted 6 lines
Quoted 2 lines
other info that might help us debug this:
Same observation here, I am releasing a syzbot report with a repro.
I guess a fix would be:
drivers/net/veth.c +0 -0
index 293dc3b2c84a6c1931e8df42cdcd5f2798004f3c..4da74ac27f9a2425d8d3f4ffcc93f453bd58e3a5
100644

drivers/net/veth.c static void veth_set_xdp_features(struct net_device *dev) · line 1268

Quoted 8 lines

drivers/net/veth.c +1 -1

@@ -1262,7 +1262,7 @@ static void veth_set_xdp_features(struct net_device *dev)
        struct veth_priv *priv = netdev_priv(dev);
        struct net_device *peer;
 
-       peer = rcu_dereference(priv->peer);
+       peer = rtnl_dereference(priv->peer);
        if (peer && peer->real_num_tx_queues <= dev->real_num_rx_queues) {
                xdp_features_t val = NETDEV_XDP_ACT_BASIC |
                                     NETDEV_XDP_ACT_REDIRECT |

Thank you for having looked!

This patch avoids the warning on our side.

Cheers,
Matt

Signature · 2 lines

Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Lorenzo Bianconi

author Acked-by

2023-03-13 16:41 UTC #11 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

In reply to Eric Dumazet

Quoted 29 lines · Eric Dumazet

Quoted 28 lines
Quoted 25 lines · Matthieu Baerts
Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################
=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!

Quoted 9 lines

Quoted 6 lines
Quoted 2 lines
other info that might help us debug this:
Same observation here, I am releasing a syzbot report with a repro.
I guess a fix would be:

Acked-by Lorenzo Bianconi <[email protected]>

Quoted 156 lines

drivers/net/veth.c +0 -0
index 293dc3b2c84a6c1931e8df42cdcd5f2798004f3c..4da74ac27f9a2425d8d3f4ffcc93f453bd58e3a5
100644
drivers/net/veth.c +1 -1
@@ -1262,7 +1262,7 @@ static void veth_set_xdp_features(struct net_device *dev)
        struct veth_priv *priv = netdev_priv(dev);
        struct net_device *peer;
 
-       peer = rcu_dereference(priv->peer);
+       peer = rtnl_dereference(priv->peer);
        if (peer && peer->real_num_tx_queues <= dev->real_num_rx_queues) {
                xdp_features_t val = NETDEV_XDP_ACT_BASIC |
                                     NETDEV_XDP_ACT_REDIRECT |
Quoted 142 lines
Quoted 142 lines
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by ip/135:
#0: ffffffff8dc4b108 (rtnl_mutex){+.+.}-{3:3}, at: rtnetlink_rcv_msg (net/core/rtnetlink.c:6172)
stack backtrace:
CPU: 1 PID: 135 Comm: ip Not tainted 6.3.0-rc1-00144-g064d70527aaa #149
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1
04/01/2014
Call Trace:
 <TASK>
dump_stack_lvl (lib/dump_stack.c:107) lockdep_rcu_suspicious (include/linux/context_tracking.h:152) veth_set_xdp_features (drivers/net/veth.c:1265 (discriminator 9)) veth_newlink (drivers/net/veth.c:1892)
? veth_set_features (drivers/net/veth.c:1774)
? kasan_save_stack (mm/kasan/common.c:47)
? kasan_save_stack (mm/kasan/common.c:46)
? kasan_set_track (mm/kasan/common.c:52)
? alloc_netdev_mqs (include/linux/slab.h:737)
? rcu_read_lock_sched_held (kernel/rcu/update.c:125)
? trace_kmalloc (include/trace/events/kmem.h:54)
? __xdp_rxq_info_reg (net/core/xdp.c:188)
? alloc_netdev_mqs (net/core/dev.c:10657)
? rtnl_create_link (net/core/rtnetlink.c:3312) rtnl_newlink_create (net/core/rtnetlink.c:3440)
? rtnl_link_get_net_capable.constprop.0 (net/core/rtnetlink.c:3391) __rtnl_newlink (net/core/rtnetlink.c:3657)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? rtnl_link_unregister (net/core/rtnetlink.c:3487) rtnl_newlink (net/core/rtnetlink.c:3671) rtnetlink_rcv_msg (net/core/rtnetlink.c:6174)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? mark_usage (kernel/locking/lockdep.c:4914)
? mark_usage (kernel/locking/lockdep.c:4914) netlink_rcv_skb (net/netlink/af_netlink.c:2574)
? rtnl_link_fill (net/core/rtnetlink.c:6070)
? netlink_ack (net/netlink/af_netlink.c:2551)
? lock_acquire (kernel/locking/lockdep.c:467)
? net_generic (include/linux/rcupdate.h:805)
? netlink_deliver_tap (include/linux/rcupdate.h:805) netlink_unicast (net/netlink/af_netlink.c:1340)
? netlink_attachskb (net/netlink/af_netlink.c:1350) netlink_sendmsg (net/netlink/af_netlink.c:1942)
? netlink_unicast (net/netlink/af_netlink.c:1861)
? netlink_unicast (net/netlink/af_netlink.c:1861) sock_sendmsg (net/socket.c:727)
____sys_sendmsg (net/socket.c:2501)
? kernel_sendmsg (net/socket.c:2448)
? __copy_msghdr (net/socket.c:2428)
___sys_sendmsg (net/socket.c:2557)
? mark_usage (kernel/locking/lockdep.c:4914)
? do_recvmmsg (net/socket.c:2544)
? lock_acquire (kernel/locking/lockdep.c:467)
? find_held_lock (kernel/locking/lockdep.c:5159)
? __lock_release (kernel/locking/lockdep.c:5345)
? __might_fault (mm/memory.c:5625)
? lock_downgrade (kernel/locking/lockdep.c:5321)
? __fget_light (include/linux/atomic/atomic-arch-fallback.h:227) __sys_sendmsg (include/linux/file.h:31)
? __sys_sendmsg_sock (net/socket.c:2572)
? rseq_get_rseq_cs (kernel/rseq.c:275)
? lockdep_hardirqs_on_prepare.part.0 (kernel/locking/lockdep.c:4263) do_syscall_64 (arch/x86/entry/common.c:50) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:120)
RIP: 0033:0x7f0d1aadeb17
Code: 0f 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b9 0f 1f 00 f3 0f 1e
fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 89 54 24 1c 48 89 74 24 10 All code
========
   0:   0f 00                   (bad)
   2:   f7 d8                   neg    %eax
   4:   64 89 02                mov    %eax,%fs:(%rdx)
   7:   48 c7 c0 ff ff ff ff    mov    $0xffffffffffffffff,%rax
   e:   eb b9                   jmp    0xffffffffffffffc9
  10:   0f 1f 00                nopl   (%rax)
  13:   f3 0f 1e fa             endbr64
  17:   64 8b 04 25 18 00 00    mov    %fs:0x18,%eax
  1e:   00
  1f:   85 c0                   test   %eax,%eax
  21:   75 10                   jne    0x33
  23:   b8 2e 00 00 00          mov    $0x2e,%eax
  28:   0f 05                   syscall
  2a:*  48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
<-- trapping instruction
  30:   77 51                   ja     0x83
  32:   c3                      ret
  33:   48 83 ec 28             sub    $0x28,%rsp
  37:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  3b:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
Code starting with the faulting instruction
===========================================
   0:   48 3d 00 f0 ff ff       cmp    $0xfffffffffffff000,%rax
   6:   77 51                   ja     0x59
   8:   c3                      ret
   9:   48 83 ec 28             sub    $0x28,%rsp
   d:   89 54 24 1c             mov    %edx,0x1c(%rsp)
  11:   48 89 74 24 10          mov    %rsi,0x10(%rsp)
RSP: 002b:00007ffca3305d48 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000640f2bb2 RCX: 00007f0d1aadeb17
RDX: 0000000000000000 RSI: 00007ffca3305db0 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000001 R09: 00007ffca3304ae0
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007ffca3305eb4 R14: 00007ffca3305e80 R15: 0000561e28bf5020
 </TASK>
ip (135) used greatest stack depth: 24544 bytes left
###################### 8< ######################
I can reproduce the issue on the "net" tree with just these 3 commands:
# ip netns add ns1
# ip netns add ns2
# ip link add ns1eth1 netns ns1 type veth peer name ns2eth1 netns ns2
Without this commit fccca038f300 ("veth: take into account device reconfiguration for xdp_features flag"), I don't have the issue.
For more details about the issue detected by CIs validating our MPTCP tree, including kconfig and vmlinux if needed:
https://github.com/multipath-tcp/mptcp_net-next/issues/372
Do you mind looking at this regression please? :)
On our side, we will revert this patch in our tree for the moment to unblock our CI jobs.
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

Eric Dumazet

2023-03-13 16:38 UTC #12 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Quoted 33 lines · Lorenzo Bianconi

Quoted 33 lines · Eric Dumazet
Quoted 31 lines
Quoted 27 lines · Matthieu Baerts
Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################
=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!
other info that might help us debug this:

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
Same observation here, I am releasing a syzbot report with a repro.
I guess a fix would be:
Acked-by Lorenzo Bianconi <[email protected]>

Can you submit a formal fix ?

Thanks.

Lorenzo Bianconi

author

2023-03-13 16:48 UTC #13 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

Quoted 38 lines · Lorenzo Bianconi

Quoted 37 lines
Quoted 37 lines · Eric Dumazet
Quoted 34 lines
Quoted 19 lines · Matthieu Baerts
Hi Lorenzo,
Quoted 6 lines · Lorenzo Bianconi
Take into account tx/rx queues reconfiguration setting device xdp_features flag. Moreover consider NETIF_F_GRO flag in order to enable ndo_xdp_xmit callback.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Signed-off-by Lorenzo Bianconi <[email protected]>
Thank you for the modification.
Unfortunately, 'git bisect' just told me this modification is the origin of a new WARN when using veth in a netns:
###################### 8< ######################
Quoted 8 lines
=============================
WARNING: suspicious RCU usage 6.3.0-rc1-00144-g064d70527aaa #149 Not tainted
-----------------------------
drivers/net/veth.c:1265 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
Same observation here, I am releasing a syzbot report with a repro.

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
I guess a fix would be:
Acked-by Lorenzo Bianconi <[email protected]>
Can you submit a formal fix ?

ack, will do.

Regards,
Lorenzo

Quoted 2 lines

Thanks.

Martin KaFai Lau

reviewer

2023-04-06 00:56 UTC #14 lore raw

Re: [PATCH net v2 6/8] veth: take into account device reconfiguration for xdp_features flag

In reply to Lorenzo Bianconi

Quoted 4 lines · Lorenzo Bianconi

drivers/net/veth.c +4 -0

+static void veth_set_xdp_features(struct net_device *dev)
+{
+	struct veth_priv *priv = netdev_priv(dev);
+	struct net_device *peer;

drivers/net/veth.c static int veth_enable_range_safe(struct net_device *dev, int start, int end) · line 1273

Quoted 10 lines

drivers/net/veth.c +10 -0

+
+	peer = rcu_dereference(priv->peer);
+	if (peer && peer->real_num_tx_queues <= dev->real_num_rx_queues) {
+		xdp_features_t val = NETDEV_XDP_ACT_BASIC |
+				     NETDEV_XDP_ACT_REDIRECT |
+				     NETDEV_XDP_ACT_RX_SG;
+
+		if (priv->_xdp_prog || veth_gro_requested(dev))
+			val |= NETDEV_XDP_ACT_NDO_XMIT |
+			       NETDEV_XDP_ACT_NDO_XMIT_SG;

This broke the xdp_do_redirect selftest. The bpf CI is consistently failing at:

test_xdp_do_redirect:FAIL:veth_src query_opts.feature_flags unexpected veth_src query_opts.feature_flags: actual 35 != expected 103

Please address it asap.

Lorenzo Bianconi

author

2023-03-09 12:27 UTC #15 lore raw

PATCH v2 7/8 net/mlx5e: take into account device reconfiguration for xdp_features flag

In reply to Lorenzo Bianconi

Take into account LRO and GRO configuration setting device xdp_features flag. Consider channel rq_wq_type enabling rx scatter-gatter support in xdp_features flag and disable NETDEV_XDP_ACT_NDO_XMIT_SG since it is not supported yet by the driver.
Moreover always enable NETDEV_XDP_ACT_NDO_XMIT as the ndo_xdp_xmit callback does not require to load a dummy xdp program on the NIC.

Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")

Co-developed-by Tariq Toukan <[email protected]>

Signed-off-by Tariq Toukan <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/mellanox/mlx5/core/en.h  |  1 +
 .../ethernet/mellanox/mlx5/core/en_ethtool.c  | 10 ++++-
 .../net/ethernet/mellanox/mlx5/core/en_main.c | 37 +++++++++++++------
 .../net/ethernet/mellanox/mlx5/core/en_rep.c  |  3 ++
 4 files changed, 39 insertions(+), 12 deletions(-)

drivers/net/ethernet/mellanox/mlx5/core/en.h +1 -0

index 88460b7796e5..4276c6eb6820 100644
@@ -1243,6 +1243,7 @@ void mlx5e_build_nic_params(struct mlx5e_priv *priv, struct mlx5e_xsk *xsk, u16
 void mlx5e_rx_dim_work(struct work_struct *work);
 void mlx5e_tx_dim_work(struct work_struct *work);
 
+void mlx5e_set_xdp_feature(struct net_device *netdev);
 netdev_features_t mlx5e_features_check(struct sk_buff *skb,
 				       struct net_device *netdev,
 				       netdev_features_t features);

drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c +9 -1

index 7708acc9b2ab..79fd21ecb9cb 100644
@@ -1985,6 +1985,7 @@ static int set_pflag_rx_striding_rq(struct net_device *netdev, bool enable)
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 	struct mlx5_core_dev *mdev = priv->mdev;
 	struct mlx5e_params new_params;
+	int err;
 
 	if (enable) {
 		/* Checking the regular RQ here; mlx5e_validate_xsk_param called

@@ -2005,7 +2006,14 @@ static int set_pflag_rx_striding_rq(struct net_device *netdev, bool enable)
 	MLX5E_SET_PFLAG(&new_params, MLX5E_PFLAG_RX_STRIDING_RQ, enable);
 	mlx5e_set_rq_type(mdev, &new_params);
 
-	return mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+	err = mlx5e_safe_switch_params(priv, &new_params, NULL, NULL, true);
+	if (err)
+		return err;
+
+	/* update XDP supported features */
+	mlx5e_set_xdp_feature(netdev);
+
+	return 0;
 }
 
 static int set_pflag_rx_no_csum_complete(struct net_device *netdev, bool enable)

drivers/net/ethernet/mellanox/mlx5/core/en_main.c +26 ~~-11~~

index 76a9c5194a70..51b5f3cca504 100644
@@ -4004,6 +4004,25 @@ static int mlx5e_handle_feature(struct net_device *netdev,
 	return 0;
 }
 
+void mlx5e_set_xdp_feature(struct net_device *netdev)
+{
+	struct mlx5e_priv *priv = netdev_priv(netdev);
+	struct mlx5e_params *params = &priv->channels.params;
+	xdp_features_t val;
+
+	if (params->packet_merge.type != MLX5E_PACKET_MERGE_NONE) {
+		xdp_clear_features_flag(netdev);
+		return;
+	}
+
+	val = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
+	      NETDEV_XDP_ACT_XSK_ZEROCOPY |
+	      NETDEV_XDP_ACT_NDO_XMIT;
+	if (params->rq_wq_type == MLX5_WQ_TYPE_CYCLIC)
+		val |= NETDEV_XDP_ACT_RX_SG;
+	xdp_set_features_flag(netdev, val);
+}
+
 int mlx5e_set_features(struct net_device *netdev, netdev_features_t features)
 {
 	netdev_features_t oper_features = features;

@@ -4030,6 +4049,9 @@ int mlx5e_set_features(struct net_device *netdev, netdev_features_t features)
 		return -EINVAL;
 	}
 
+	/* update XDP supported features */
+	mlx5e_set_xdp_feature(netdev);
+
 	return 0;
 }

@@ -4761,13 +4783,6 @@ static int mlx5e_xdp_set(struct net_device *netdev, struct bpf_prog *prog)
 	if (old_prog)
 		bpf_prog_put(old_prog);
 
-	if (reset) {
-		if (prog)
-			xdp_features_set_redirect_target(netdev, true);
-		else
-			xdp_features_clear_redirect_target(netdev);
-	}
-
 	if (!test_bit(MLX5E_STATE_OPENED, &priv->state) || reset)
 		goto unlock;

@@ -5163,13 +5178,10 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
 	netdev->features         |= NETIF_F_HIGHDMA;
 	netdev->features         |= NETIF_F_HW_VLAN_STAG_FILTER;
 
-	netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-			       NETDEV_XDP_ACT_XSK_ZEROCOPY |
-			       NETDEV_XDP_ACT_RX_SG;
-
 	netdev->priv_flags       |= IFF_UNICAST_FLT;
 
 	netif_set_tso_max_size(netdev, GSO_MAX_SIZE);
+	mlx5e_set_xdp_feature(netdev);
 	mlx5e_set_netdev_dev_addr(netdev);
 	mlx5e_macsec_build_netdev(priv);
 	mlx5e_ipsec_build_netdev(priv);

@@ -5241,6 +5253,9 @@ static int mlx5e_nic_init(struct mlx5_core_dev *mdev,
 		mlx5_core_err(mdev, "TLS initialization failed, %d\n", err);
 
 	mlx5e_health_create_reporters(priv);
+	/* update XDP supported features */
+	mlx5e_set_xdp_feature(netdev);
+
 	return 0;
 }

drivers/net/ethernet/mellanox/mlx5/core/en_rep.c +3 -0

index 9b9203443085..43fd12fb87b8 100644
@@ -747,6 +747,9 @@ static void mlx5e_build_rep_params(struct net_device *netdev)
 	/* RQ */
 	mlx5e_build_rq_params(mdev, params);
 
+	/* update XDP supported features */
+	mlx5e_set_xdp_feature(netdev);
+
 	/* CQ moderation params */
 	params->rx_dim_enabled = MLX5_CAP_GEN(mdev, cq_moderation);
 	mlx5e_set_rx_cq_mode_params(params, cq_period_mode);

Signature · 1 line

2.39.2

Jakub Kicinski

maintainer

2023-03-15 23:45 UTC #16 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 11 lines · On Thu, 9 Mar 2023 13:25:31 +0100 Lorenzo Bianconi wrote:

Take into account LRO and GRO configuration setting device xdp_features flag. Consider channel rq_wq_type enabling rx scatter-gatter support in xdp_features flag and disable NETDEV_XDP_ACT_NDO_XMIT_SG since it is not supported yet by the driver.
Moreover always enable NETDEV_XDP_ACT_NDO_XMIT as the ndo_xdp_xmit callback does not require to load a dummy xdp program on the NIC.
Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")
Co-developed-by Tariq Toukan <[email protected]>
Signed-off-by Tariq Toukan <[email protected]>
Signed-off-by Lorenzo Bianconi <[email protected]>

This one hits ASSERT_RTNL(), I think. Don't we need something like:

net/core/xdp.c +3 -0

index 87e654b7d06c..5722a1fc6e9e 100644
@@ -781,6 +781,9 @@ void xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
                return;
 
        dev->xdp_features = val;
+
+       if (dev->reg_state < NETREG_REGISTERED)
+               return;
        call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev);
 }
 EXPORT_SYMBOL_GPL(xdp_set_features_flag);

? The notifiers are not needed until the device is actually live.

Jakub Kicinski

maintainer

2023-03-16 00:29 UTC #17 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 15 lines · On Wed, 15 Mar 2023 16:39:00 -0700 Jakub Kicinski wrote:

net/core/xdp.c +3 -0

index 87e654b7d06c..5722a1fc6e9e 100644
@@ -781,6 +781,9 @@ void xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
                return;
 
        dev->xdp_features = val;
+
+       if (dev->reg_state < NETREG_REGISTERED)
+               return;
        call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev);
 }
 EXPORT_SYMBOL_GPL(xdp_set_features_flag);

Quoted 1 line

? The notifiers are not needed until the device is actually live.

I think so.. let me send a full patch.

Saeed Mahameed

2023-03-16 02:46 UTC #18 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 16 lines · Jakub Kicinski

Quoted 15 lines · On Wed, 15 Mar 2023 16:39:00 -0700 Jakub Kicinski wrote:

net/core/xdp.c +3 -0

index 87e654b7d06c..5722a1fc6e9e 100644
@@ -781,6 +781,9 @@ void xdp_set_features_flag(struct net_device *dev, xdp_features_t val)
                return;
 
        dev->xdp_features = val;
+
+       if (dev->reg_state < NETREG_REGISTERED)
+               return;
        call_netdevice_notifiers(NETDEV_XDP_FEAT_CHANGE, dev);
 }
 EXPORT_SYMBOL_GPL(xdp_set_features_flag);

Quoted 3 lines

Quoted 1 line
? The notifiers are not needed until the device is actually live.
I think so.. let me send a full patch.

We have an internal version of a fix, Tariq is finalizing some review comments and we will be posting it ASAP.

Jakub Kicinski

maintainer

2023-03-16 02:53 UTC #19 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 4 lines · On Wed, 15 Mar 2023 19:46:10 -0700 Saeed Mahameed wrote:

Quoted 1 line
I think so.. let me send a full patch.
We have an internal version of a fix, Tariq is finalizing some review comments and we will be posting it ASAP.

Ah, I already posted. Does it look different?

https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

I wanted to make sure that it's ready for tomorrow's PR

Saeed Mahameed

2023-03-16 05:03 UTC #20 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 8 lines · Jakub Kicinski

Quoted 4 lines · On Wed, 15 Mar 2023 19:46:10 -0700 Saeed Mahameed wrote:
Quoted 1 line
I think so.. let me send a full patch.
We have an internal version of a fix, Tariq is finalizing some review comments and we will be posting it ASAP.
Ah, I already posted. Does it look different?

Yes, completely different, Tariq's fix is in mlx5 only.
He splits the xdp feature setting into two functions, one to initialize the netdev's xdp before registration, and another one to update xpd features and call the notifier in the
"after" registration set_features flows.

I like our solution more, since it's more explicit and doesn't require patching xdp stack because mlx5 abused xdp_set_features_flag, unless other drivers have the same issue.

Quoted 2 lines

https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

I don't see anything wrong with your patch though.. it also looks more elegant, i dunno, I will let you decide, here's our patch:
https://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git/commit/?h=testing/xdp-features-fix&id=72e2266525948ba1498e6a3f2d63ea10d5ee86f5

Quoted 1 line

I wanted to make sure that it's ready for tomorrow's PR

Jakub Kicinski

maintainer

2023-03-16 05:37 UTC #21 lore raw

Re: [PATCH net v2 7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag

Quoted 9 lines · On Wed, 15 Mar 2023 22:03:27 -0700 Saeed Mahameed wrote:

Yes, completely different, Tariq's fix is in mlx5 only. He splits the xdp feature setting into two functions, one to initialize the netdev's xdp before registration, and another one to update xpd features and call the notifier in the "after" registration set_features flows.
I like our solution more, since it's more explicit and doesn't require patching xdp stack because mlx5 abused xdp_set_features_flag, unless other drivers have the same issue.

I reckon there's nothing wrong with calling xdp_set_features_flag() before registration, I didn't do much research, but netif_set_real_num_*x_queues() come to mind, and they can be called at any time. The stack should act accordingly.

Many drivers may need to work around an issue which can be handled centrally.

Let's see if Lorenzo disagrees.

Quoted 6 lines

Quoted 2 lines
https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
I don't see anything wrong with your patch though.. it also looks more elegant, i dunno, I will let you decide, here's our patch:
https://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git/commit/?h=testing/xdp-features-fix&id=72e2266525948ba1498e6a3f2d63ea10d5ee86f5

Lorenzo Bianconi

author

2023-03-09 12:27 UTC #22 lore raw

PATCH v2 8/8 mvpp2: take care of xdp_features when reconfiguring queues

In reply to Lorenzo Bianconi

From: Matteo Croce <[email protected]>

XDP is supported only if enough queues are present, so when reconfiguring the queues set xdp_features accordingly.

Fixes 66c0e13ad236 ("drivers: net: turn on XDP features")

Signed-off-by Matteo Croce <[email protected]>

Signed-off-by Lorenzo Bianconi <[email protected]>

---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +12 -3

index 9b4ecbe4f36d..3ea00bc9b91c 100644
@@ -4996,6 +4996,14 @@ static int mvpp2_bm_switch_buffers(struct mvpp2 *priv, bool percpu)
 
 	for (i = 0; i < priv->port_count; i++) {
 		port = priv->port_list[i];
+		if (percpu && port->ntxqs >= num_possible_cpus() * 2)
+			xdp_set_features_flag(port->dev,
+					      NETDEV_XDP_ACT_BASIC |
+					      NETDEV_XDP_ACT_REDIRECT |
+					      NETDEV_XDP_ACT_NDO_XMIT);
+		else
+			xdp_clear_features_flag(port->dev);
+
 		mvpp2_swf_bm_pool_init(port);
 		if (status[i])
 			mvpp2_open(port->dev);

@@ -6863,13 +6871,14 @@ static int mvpp2_port_probe(struct platform_device *pdev,
 
 	if (!port->priv->percpu_pools)
 		mvpp2_set_hw_csum(port, port->pool_long->id);
+	else if (port->ntxqs >= num_possible_cpus() * 2)
+		dev->xdp_features = NETDEV_XDP_ACT_BASIC |
+				    NETDEV_XDP_ACT_REDIRECT |
+				    NETDEV_XDP_ACT_NDO_XMIT;
 
 	dev->vlan_features |= features;
 	netif_set_tso_max_segs(dev, MVPP2_MAX_TSO_SEGS);
 
-	dev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT |
-			    NETDEV_XDP_ACT_NDO_XMIT;
-
 	dev->priv_flags |= IFF_UNICAST_FLT;
 
 	/* MTU range: 68 - 9704 */

Signature · 1 line

2.39.2

[email protected]

CI Applied

2023-03-11 05:50 UTC #23 lore raw

In reply to Lorenzo Bianconi

This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 15 lines · On Thu, 9 Mar 2023 13:25:24 +0100 you wrote:

Changes since v1:
- rebase on top of net tree
- remove NETDEV_XDP_ACT_NDO_XMIT_SG support in mlx5e driver
- always enable NETDEV_XDP_ACT_NDO_XMIT support in mlx5e driver
Lorenzo Bianconi (7):
tools: ynl: fix render-max for flags definition
tools: ynl: fix get_mask utility routine
xdp: add xdp_set_features_flag utility routine
net: thunderx: take into account xdp_features setting tx/rx queues
net: ena: take into account xdp_features setting tx/rx queues
veth: take into account device reconfiguration for xdp_features flag net/mlx5e: take into account device reconfiguration for xdp_features
flag

Quoted 1 line

[...]

Here is the summary with links:
- [net,v2,1/8] tools: ynl: fix render-max for flags definition

    https://git.kernel.org/netdev/net/c/8f76a4f80fba
  - [net,v2,2/8] tools: ynl: fix get_mask utility routine
    https://git.kernel.org/netdev/net/c/bf51d27704c9
  - [net,v2,3/8] xdp: add xdp_set_features_flag utility routine
    https://git.kernel.org/netdev/net/c/f85949f98206
  - [net,v2,4/8] net: thunderx: take into account xdp_features setting tx/rx queues
    https://git.kernel.org/netdev/net/c/3c249fe4de16
  - [net,v2,5/8] net: ena: take into account xdp_features setting tx/rx queues
    https://git.kernel.org/netdev/net/c/7aa6dc351b92
  - [net,v2,6/8] veth: take into account device reconfiguration for xdp_features flag
    https://git.kernel.org/netdev/net/c/fccca038f300
  - [net,v2,7/8] net/mlx5e: take into account device reconfiguration for xdp_features flag
    https://git.kernel.org/netdev/net/c/4d5ab0ad964d
  - [net,v2,8/8] mvpp2: take care of xdp_features when reconfiguring queues
    https://git.kernel.org/netdev/net/c/481e96fc1307

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html