From 7fb1073300a2ea8bd03b2fc7d5a591192e48ea24 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Mon, 6 Jan 2025 16:07:51 +0900 Subject: [PATCH 01/16] net: Hold rtnl_net_lock() in (un)?register_netdevice_notifier_dev_net(). (un)?register_netdevice_notifier_dev_net() hold RTNL before triggering the notifier for all netdev in the netns. Let's convert the RTNL to rtnl_net_lock(). Note that move_netdevice_notifiers_dev_net() is assumed to be (but not yet) protected by per-netns RTNL of both src and dst netns; we need to convert wireless and hyperv drivers that call dev_change_net_namespace(). Signed-off-by: Kuniyuki Iwashima Link: https://patch.msgid.link/20250106070751.63146-4-kuniyu@amazon.com Signed-off-by: Jakub Kicinski --- net/core/dev.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/net/core/dev.c b/net/core/dev.c index 06a5c11688c1..efbe2c4d9458 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -1946,15 +1946,17 @@ int register_netdevice_notifier_dev_net(struct net_device *dev, struct notifier_block *nb, struct netdev_net_notifier *nn) { + struct net *net = dev_net(dev); int err; - rtnl_lock(); - err = __register_netdevice_notifier_net(dev_net(dev), nb, false); + rtnl_net_lock(net); + err = __register_netdevice_notifier_net(net, nb, false); if (!err) { nn->nb = nb; list_add(&nn->list, &dev->net_notifier_list); } - rtnl_unlock(); + rtnl_net_unlock(net); + return err; } EXPORT_SYMBOL(register_netdevice_notifier_dev_net); @@ -1963,12 +1965,14 @@ int unregister_netdevice_notifier_dev_net(struct net_device *dev, struct notifier_block *nb, struct netdev_net_notifier *nn) { + struct net *net = dev_net(dev); int err; - rtnl_lock(); + rtnl_net_lock(net); list_del(&nn->list); - err = __unregister_netdevice_notifier_net(dev_net(dev), nb); - rtnl_unlock(); + err = __unregister_netdevice_notifier_net(net, nb); + rtnl_net_unlock(net); + return err; } EXPORT_SYMBOL(unregister_netdevice_notifier_dev_net); -- 2.51.0 From 0945a7b442200f908a8e2f67ba22ccfa77b3598e Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:58:43 +0000 Subject: [PATCH 02/16] net: dsa: ksz: remove setting of tx_lpi parameters dsa_user_get_eee() calls the DSA switch get_mac_eee() method followed by phylink_ethtool_get_eee(), which goes on to call phy_ethtool_get_eee(). This overwrites all members of the passed ethtool_keee, which means anything written by the DSA switch get_mac_eee() method will be discarded. Remove setting any members in ksz_get_mac_eee(). Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUlkp-007UyW-OR@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/microchip/ksz_common.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index e3512e324572..60a4630dd7ba 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -3492,14 +3492,6 @@ static bool ksz_support_eee(struct dsa_switch *ds, int port) static int ksz_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { - /* There is no documented control of Tx LPI configuration. */ - e->tx_lpi_enabled = true; - - /* There is no documented control of Tx LPI timer. According to tests - * Tx LPI timer seems to be set by default to minimal value. - */ - e->tx_lpi_timer = 0; - return 0; } -- 2.51.0 From 22cedc609759edf414c519ae8242dd5461f4c4cd Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:58:48 +0000 Subject: [PATCH 03/16] net: dsa: mt753x: remove setting of tx_lpi parameters dsa_user_get_eee() calls the DSA switch get_mac_eee() method followed by phylink_ethtool_get_eee(), which goes on to call phy_ethtool_get_eee(). This overwrites all members of the passed ethtool_keee, which means anything written by the DSA switch get_mac_eee() method will be discarded. Remove setting any members in mt753x_get_mac_eee(). Signed-off-by: Russell King (Oracle) Reviewed-by: Chester A. Unal Link: https://patch.msgid.link/E1tUlku-007Uyc-RP@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 9605febd3573..4c78d049b9f4 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -3088,12 +3088,6 @@ mt753x_setup(struct dsa_switch *ds) static int mt753x_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { - struct mt7530_priv *priv = ds->priv; - u32 eeecr = mt7530_read(priv, MT753X_PMEEECR_P(port)); - - e->tx_lpi_enabled = !(eeecr & LPI_MODE_EN); - e->tx_lpi_timer = LPI_THRESH_GET(eeecr); - return 0; } -- 2.51.0 From 60c6e3a59299361a1057b0b61d8b0495f6eb7cf7 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:58:53 +0000 Subject: [PATCH 04/16] net: dsa: no longer call ds->ops->get_mac_eee() All implementations of get_mac_eee() now just return zero without doing anything useful. Remove the call to this method in preparation to removing the method from each DSA driver. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUlkz-007Uyl-UA@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- net/dsa/user.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/net/dsa/user.c b/net/dsa/user.c index 4a8de48a6f24..c74f2b2b92de 100644 --- a/net/dsa/user.c +++ b/net/dsa/user.c @@ -1251,7 +1251,6 @@ static int dsa_user_get_eee(struct net_device *dev, struct ethtool_keee *e) { struct dsa_port *dp = dsa_user_to_port(dev); struct dsa_switch *ds = dp->ds; - int ret; /* Check whether the switch supports EEE */ if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index)) @@ -1261,13 +1260,6 @@ static int dsa_user_get_eee(struct net_device *dev, struct ethtool_keee *e) if (!dev->phydev) return -ENODEV; - if (!ds->ops->get_mac_eee) - return -EOPNOTSUPP; - - ret = ds->ops->get_mac_eee(ds, dp->index, e); - if (ret) - return ret; - return phylink_ethtool_get_eee(dp->pl, e); } -- 2.51.0 From 08cef9e1b08313f1ad0fdc16340e55bc75a5e2dc Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:58:59 +0000 Subject: [PATCH 05/16] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee() b53_get_mac_eee() is no longer called by the core DSA code. Remove it. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUll5-007Uyr-1U@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/b53/b53_common.c | 7 ------- drivers/net/dsa/b53/b53_priv.h | 1 - drivers/net/dsa/bcm_sf2.c | 1 - 3 files changed, 9 deletions(-) diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c index 0561b60f668f..79dc77835681 100644 --- a/drivers/net/dsa/b53/b53_common.c +++ b/drivers/net/dsa/b53/b53_common.c @@ -2232,12 +2232,6 @@ bool b53_support_eee(struct dsa_switch *ds, int port) } EXPORT_SYMBOL(b53_support_eee); -int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) -{ - return 0; -} -EXPORT_SYMBOL(b53_get_mac_eee); - int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { struct b53_device *dev = ds->priv; @@ -2299,7 +2293,6 @@ static const struct dsa_switch_ops b53_switch_ops = { .port_enable = b53_enable_port, .port_disable = b53_disable_port, .support_eee = b53_support_eee, - .get_mac_eee = b53_get_mac_eee, .set_mac_eee = b53_set_mac_eee, .port_bridge_join = b53_br_join, .port_bridge_leave = b53_br_leave, diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h index 99e5cfc98ae8..9e9b5bc0c5d6 100644 --- a/drivers/net/dsa/b53/b53_priv.h +++ b/drivers/net/dsa/b53/b53_priv.h @@ -385,7 +385,6 @@ void b53_disable_port(struct dsa_switch *ds, int port); void b53_brcm_hdr_setup(struct dsa_switch *ds, int port); int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy); bool b53_support_eee(struct dsa_switch *ds, int port); -int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e); int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e); #endif diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c index a53fb6191e6b..fa2bf3fa9019 100644 --- a/drivers/net/dsa/bcm_sf2.c +++ b/drivers/net/dsa/bcm_sf2.c @@ -1233,7 +1233,6 @@ static const struct dsa_switch_ops bcm_sf2_ops = { .port_enable = bcm_sf2_port_setup, .port_disable = bcm_sf2_port_disable, .support_eee = b53_support_eee, - .get_mac_eee = b53_get_mac_eee, .set_mac_eee = b53_set_mac_eee, .port_bridge_join = b53_br_join, .port_bridge_leave = b53_br_leave, -- 2.51.0 From e2d1b8090b69af793178713f1851706394739410 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:59:04 +0000 Subject: [PATCH 06/16] net: dsa: ksz: remove ksz_get_mac_eee() ksz_get_mac_eee() is no longer called by the core DSA code. Remove it. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUllA-007Uyx-4o@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/microchip/ksz_common.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c index 60a4630dd7ba..89f0796894af 100644 --- a/drivers/net/dsa/microchip/ksz_common.c +++ b/drivers/net/dsa/microchip/ksz_common.c @@ -3489,12 +3489,6 @@ static bool ksz_support_eee(struct dsa_switch *ds, int port) return false; } -static int ksz_get_mac_eee(struct dsa_switch *ds, int port, - struct ethtool_keee *e) -{ - return 0; -} - static int ksz_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { @@ -4664,7 +4658,6 @@ static const struct dsa_switch_ops ksz_switch_ops = { .cls_flower_del = ksz_cls_flower_del, .port_setup_tc = ksz_setup_tc, .support_eee = ksz_support_eee, - .get_mac_eee = ksz_get_mac_eee, .set_mac_eee = ksz_set_mac_eee, .port_get_default_prio = ksz_port_get_default_prio, .port_set_default_prio = ksz_port_set_default_prio, -- 2.51.0 From 9e66e8ebe7a98be3f7ce5c4c5702f53b96677784 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:59:09 +0000 Subject: [PATCH 07/16] net: dsa: mt753x: remove ksz_get_mac_eee() mt753x_get_mac_eee() is no longer called by the core DSA code. Remove it. Signed-off-by: Russell King (Oracle) Reviewed-by: Chester A. Unal Link: https://patch.msgid.link/E1tUllF-007Uz3-95@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mt7530.c | 7 ------- 1 file changed, 7 deletions(-) diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c index 4c78d049b9f4..d2d0f091e49e 100644 --- a/drivers/net/dsa/mt7530.c +++ b/drivers/net/dsa/mt7530.c @@ -3085,12 +3085,6 @@ mt753x_setup(struct dsa_switch *ds) return ret; } -static int mt753x_get_mac_eee(struct dsa_switch *ds, int port, - struct ethtool_keee *e) -{ - return 0; -} - static int mt753x_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { @@ -3233,7 +3227,6 @@ const struct dsa_switch_ops mt7530_switch_ops = { .port_mirror_del = mt753x_port_mirror_del, .phylink_get_caps = mt753x_phylink_get_caps, .support_eee = dsa_supports_eee, - .get_mac_eee = mt753x_get_mac_eee, .set_mac_eee = mt753x_set_mac_eee, .conduit_state_change = mt753x_conduit_state_change, .port_setup_tc = mt753x_setup_tc, -- 2.51.0 From d3889a3d1351aa4990b7b70d2c3da73a7b690b3f Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:59:14 +0000 Subject: [PATCH 08/16] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee() mv88e6xxx_get_mac_eee() is no longer called by the core DSA code. Remove it. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUllK-007Uz9-D7@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/mv88e6xxx/chip.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c index 570c8642d387..35ae084af166 100644 --- a/drivers/net/dsa/mv88e6xxx/chip.c +++ b/drivers/net/dsa/mv88e6xxx/chip.c @@ -1513,13 +1513,6 @@ static void mv88e6xxx_get_regs(struct dsa_switch *ds, int port, mv88e6xxx_reg_unlock(chip); } -static int mv88e6xxx_get_mac_eee(struct dsa_switch *ds, int port, - struct ethtool_keee *e) -{ - /* Nothing to do on the port's MAC */ - return 0; -} - static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e) { @@ -7100,7 +7093,6 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = { .port_max_mtu = mv88e6xxx_get_max_mtu, .port_change_mtu = mv88e6xxx_change_mtu, .support_eee = dsa_supports_eee, - .get_mac_eee = mv88e6xxx_get_mac_eee, .set_mac_eee = mv88e6xxx_set_mac_eee, .get_eeprom_len = mv88e6xxx_get_eeprom_len, .get_eeprom = mv88e6xxx_get_eeprom, -- 2.51.0 From d19be79a67b32b74a436e7cb2a712f3c75c5ea35 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:59:19 +0000 Subject: [PATCH 09/16] net: dsa: qca: remove qca8k_get_mac_eee() qca8k_get_mac_eee() is no longer called by the core DSA code. Remove it. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUllP-007UzF-Gk@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- drivers/net/dsa/qca/qca8k-8xxx.c | 1 - drivers/net/dsa/qca/qca8k-common.c | 7 ------- drivers/net/dsa/qca/qca8k.h | 1 - 3 files changed, 9 deletions(-) diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c index 90e24bc00b99..2d56a8152420 100644 --- a/drivers/net/dsa/qca/qca8k-8xxx.c +++ b/drivers/net/dsa/qca/qca8k-8xxx.c @@ -2017,7 +2017,6 @@ static const struct dsa_switch_ops qca8k_switch_ops = { .get_sset_count = qca8k_get_sset_count, .set_ageing_time = qca8k_set_ageing_time, .support_eee = dsa_supports_eee, - .get_mac_eee = qca8k_get_mac_eee, .set_mac_eee = qca8k_set_mac_eee, .port_enable = qca8k_port_enable, .port_disable = qca8k_port_disable, diff --git a/drivers/net/dsa/qca/qca8k-common.c b/drivers/net/dsa/qca/qca8k-common.c index 560c74c4ac3d..13005f10edb7 100644 --- a/drivers/net/dsa/qca/qca8k-common.c +++ b/drivers/net/dsa/qca/qca8k-common.c @@ -557,13 +557,6 @@ exit: return ret; } -int qca8k_get_mac_eee(struct dsa_switch *ds, int port, - struct ethtool_keee *e) -{ - /* Nothing to do on the port's MAC */ - return 0; -} - static int qca8k_port_configure_learning(struct dsa_switch *ds, int port, bool learning) { diff --git a/drivers/net/dsa/qca/qca8k.h b/drivers/net/dsa/qca/qca8k.h index 24962a395754..d046679265fa 100644 --- a/drivers/net/dsa/qca/qca8k.h +++ b/drivers/net/dsa/qca/qca8k.h @@ -520,7 +520,6 @@ int qca8k_get_sset_count(struct dsa_switch *ds, int port, int sset); /* Common eee function */ int qca8k_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *eee); -int qca8k_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_keee *e); /* Common bridge function */ void qca8k_port_stp_state_set(struct dsa_switch *ds, int port, u8 state); -- 2.51.0 From 2fa8b4383d24c1c788528ed4377d9f07c6c10227 Mon Sep 17 00:00:00 2001 From: "Russell King (Oracle)" Date: Mon, 6 Jan 2025 11:59:24 +0000 Subject: [PATCH 10/16] net: dsa: remove get_mac_eee() method The get_mac_eee() is no longer called by the core DSA code, nor are there any implementations of this method. Remove it. Signed-off-by: Russell King (Oracle) Link: https://patch.msgid.link/E1tUllU-007UzL-KV@rmk-PC.armlinux.org.uk Signed-off-by: Jakub Kicinski --- include/net/dsa.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/net/dsa.h b/include/net/dsa.h index 4aeedb296d67..9640d5c67f56 100644 --- a/include/net/dsa.h +++ b/include/net/dsa.h @@ -991,8 +991,6 @@ struct dsa_switch_ops { bool (*support_eee)(struct dsa_switch *ds, int port); int (*set_mac_eee)(struct dsa_switch *ds, int port, struct ethtool_keee *e); - int (*get_mac_eee)(struct dsa_switch *ds, int port, - struct ethtool_keee *e); /* EEPROM access */ int (*get_eeprom_len)(struct dsa_switch *ds); -- 2.51.0 From d8c2e5f33acec38cf478c509c65646d029cc378e Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 6 Jan 2025 09:46:20 -0800 Subject: [PATCH 11/16] if_vlan: fix kdoc warnings While merging net to net-next I noticed that the kdoc above __vlan_get_protocol_offset() has the wrong function name. Fix that and all the other kdoc warnings in this file. Reviewed-by: Simon Horman Link: https://patch.msgid.link/20250106174620.1855269-1-kuba@kernel.org Signed-off-by: Jakub Kicinski --- include/linux/if_vlan.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h index d495cbdb52cb..38456b42cdb5 100644 --- a/include/linux/if_vlan.h +++ b/include/linux/if_vlan.h @@ -176,6 +176,7 @@ struct netpoll; * @real_dev_addr: address of underlying netdevice * @dent: proc dir entry * @vlan_pcpu_stats: ptr to percpu rx stats + * @netpoll: netpoll instance "propagated" down to @real_dev */ struct vlan_dev_priv { unsigned int nr_ingress_mappings; @@ -414,6 +415,8 @@ static inline int __vlan_insert_tag(struct sk_buff *skb, * doesn't have to worry about freeing the original skb. * * Does not change skb->protocol so this function can be used during receive. + * + * Return: modified @skb on success, NULL on error (@skb is freed). */ static inline struct sk_buff *vlan_insert_inner_tag(struct sk_buff *skb, __be16 vlan_proto, @@ -443,6 +446,8 @@ static inline struct sk_buff *vlan_insert_inner_tag(struct sk_buff *skb, * doesn't have to worry about freeing the original skb. * * Does not change skb->protocol so this function can be used during receive. + * + * Return: modified @skb on success, NULL on error (@skb is freed). */ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, __be16 vlan_proto, u16 vlan_tci) @@ -461,6 +466,8 @@ static inline struct sk_buff *vlan_insert_tag(struct sk_buff *skb, * * Following the skb_unshare() example, in case of error, the calling function * doesn't have to worry about freeing the original skb. + * + * Return: modified @skb on success, NULL on error (@skb is freed). */ static inline struct sk_buff *vlan_insert_tag_set_proto(struct sk_buff *skb, __be16 vlan_proto, @@ -582,7 +589,7 @@ static inline int vlan_get_tag(const struct sk_buff *skb, u16 *vlan_tci) } /** - * vlan_get_protocol - get protocol EtherType. + * __vlan_get_protocol_offset() - get protocol EtherType. * @skb: skbuff to query * @type: first vlan protocol * @mac_offset: MAC offset @@ -808,9 +815,11 @@ static inline netdev_features_t vlan_features_check(struct sk_buff *skb, * @h1: Pointer to vlan header * @h2: Pointer to vlan header * - * Compare two vlan headers, returns 0 if equal. + * Compare two vlan headers. * * Please note that alignment of h1 & h2 are only guaranteed to be 16 bits. + * + * Return: 0 if equal, arbitrary non-zero value if not equal. */ static inline unsigned long compare_vlan_header(const struct vlan_hdr *h1, const struct vlan_hdr *h2) -- 2.51.0 From 69072db934dfc7a566d4eb1fac04146e97ab365f Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 6 Jan 2025 18:28:18 -0800 Subject: [PATCH 12/16] tools: ynl: correctly handle overrides of fields in subset We stated in documentation [1] and previous discussions [2] that the need for overriding fields in members of subsets is anticipated. Implement it. Since each attr is now a new object we need to make sure that the modifications are propagated. Specifically C codegen wants to annotate which attrs are used in requests and replies to generate the right validation artifacts. [1] https://docs.kernel.org/next/userspace-api/netlink/specs.html#subset-of [2] https://lore.kernel.org/netdev/20231004171350.1f59cd1d@kernel.org/ Reviewed-by: Donald Hunter Link: https://patch.msgid.link/20250107022820.2087101-2-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/nlspec.py | 5 ++++- tools/net/ynl/ynl-gen-c.py | 26 ++++++++++++++++++++++---- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/tools/net/ynl/lib/nlspec.py b/tools/net/ynl/lib/nlspec.py index a745739655ad..314ec8007496 100644 --- a/tools/net/ynl/lib/nlspec.py +++ b/tools/net/ynl/lib/nlspec.py @@ -219,7 +219,10 @@ class SpecAttrSet(SpecElement): else: real_set = family.attr_sets[self.subset_of] for elem in self.yaml['attributes']: - attr = real_set[elem['name']] + real_attr = real_set[elem['name']] + combined_elem = real_attr.yaml | elem + attr = self.new_attr(combined_elem, real_attr.value) + self.attrs[attr.name] = attr self.attrs_by_val[attr.value] = attr diff --git a/tools/net/ynl/ynl-gen-c.py b/tools/net/ynl/ynl-gen-c.py index ec2288948795..58657dd7dedb 100755 --- a/tools/net/ynl/ynl-gen-c.py +++ b/tools/net/ynl/ynl-gen-c.py @@ -79,6 +79,20 @@ class Type(SpecAttr): self.enum_name = None delattr(self, "enum_name") + def _get_real_attr(self): + # if the attr is for a subset return the "real" attr (just one down, does not recurse) + return self.family.attr_sets[self.attr_set.subset_of][self.name] + + def set_request(self): + self.request = True + if self.attr_set.subset_of: + self._get_real_attr().set_request() + + def set_reply(self): + self.reply = True + if self.attr_set.subset_of: + self._get_real_attr().set_reply() + def get_limit(self, limit, default=None): value = self.checks.get(limit, default) if value is None: @@ -106,6 +120,10 @@ class Type(SpecAttr): enum_name = f"{self.attr_set.name_prefix}{self.name}" self.enum_name = c_upper(enum_name) + if self.attr_set.subset_of: + if self.checks != self._get_real_attr().checks: + raise Exception("Overriding checks not supported by codegen, yet") + def is_multi_val(self): return None @@ -1119,17 +1137,17 @@ class Family(SpecFamily): for _, struct in self.pure_nested_structs.items(): if struct.request: for _, arg in struct.member_list(): - arg.request = True + arg.set_request() if struct.reply: for _, arg in struct.member_list(): - arg.reply = True + arg.set_reply() for root_set, rs_members in self.root_sets.items(): for attr, spec in self.attr_sets[root_set].items(): if attr in rs_members['request']: - spec.request = True + spec.set_request() if attr in rs_members['reply']: - spec.reply = True + spec.set_reply() def _load_global_policy(self): global_set = set() -- 2.51.0 From 7aae6505351e4c9fc2f3108d16fd06ce76f4b475 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 6 Jan 2025 18:28:19 -0800 Subject: [PATCH 13/16] tools: ynl: print some information about attribute we can't parse When parsing throws an exception one often has to figure out which attribute couldn't be parsed from first principles. For families with large message parsing trees like rtnetlink guessing the attribute can be hard. Print a bit of information as the exception travels out, e.g.: # when dumping rt links Error decoding 'flags' from 'linkinfo-ip6tnl-attrs' Error decoding 'data' from 'linkinfo-attrs' Error decoding 'linkinfo' from 'link-attrs' Traceback (most recent call last): File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 119, in main() File "/home/kicinski/linux/./tools/net/ynl/cli.py", line 100, in main reply = ynl.dump(args.dump, attrs) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1064, in dump return self._op(method, vals, dump=True) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1058, in _op return self._ops(ops)[0] File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 1045, in _ops rsp_msg = self._decode(decoded.raw_attrs, op.attr_set.name) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 738, in _decode subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 763, in _decode decoded = self._decode_sub_msg(attr, attr_spec, search_attrs) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 714, in _decode_sub_msg subdict = self._decode(NlAttrs(attr.raw, offset), msg_format.attr_set) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 749, in _decode decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order) File "/home/kicinski/linux/tools/net/ynl/lib/ynl.py", line 147, in as_scalar return format.unpack(self.raw)[0] struct.error: unpack requires a buffer of 2 bytes The Traceback is what we would previously see, the "Error..." messages are new. We print a message per level (in the stack order). Printing single combined message gets tricky quickly given sub-messages etc. Reviewed-by: Donald Hunter Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20250107022820.2087101-3-kuba@kernel.org Signed-off-by: Jakub Kicinski --- tools/net/ynl/lib/ynl.py | 74 +++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/tools/net/ynl/lib/ynl.py b/tools/net/ynl/lib/ynl.py index eea29359a899..08f8bf89cfc2 100644 --- a/tools/net/ynl/lib/ynl.py +++ b/tools/net/ynl/lib/ynl.py @@ -733,41 +733,45 @@ class YnlFamily(SpecFamily): self._rsp_add(rsp, attr_name, None, self._decode_unknown(attr)) continue - if attr_spec["type"] == 'nest': - subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs) - decoded = subdict - elif attr_spec["type"] == 'string': - decoded = attr.as_strz() - elif attr_spec["type"] == 'binary': - decoded = self._decode_binary(attr, attr_spec) - elif attr_spec["type"] == 'flag': - decoded = True - elif attr_spec.is_auto_scalar: - decoded = attr.as_auto_scalar(attr_spec['type'], attr_spec.byte_order) - elif attr_spec["type"] in NlAttr.type_formats: - decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order) - if 'enum' in attr_spec: - decoded = self._decode_enum(decoded, attr_spec) - elif attr_spec.display_hint: - decoded = self._formatted_string(decoded, attr_spec.display_hint) - elif attr_spec["type"] == 'indexed-array': - decoded = self._decode_array_attr(attr, attr_spec) - elif attr_spec["type"] == 'bitfield32': - value, selector = struct.unpack("II", attr.raw) - if 'enum' in attr_spec: - value = self._decode_enum(value, attr_spec) - selector = self._decode_enum(selector, attr_spec) - decoded = {"value": value, "selector": selector} - elif attr_spec["type"] == 'sub-message': - decoded = self._decode_sub_msg(attr, attr_spec, search_attrs) - elif attr_spec["type"] == 'nest-type-value': - decoded = self._decode_nest_type_value(attr, attr_spec) - else: - if not self.process_unknown: - raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}') - decoded = self._decode_unknown(attr) - - self._rsp_add(rsp, attr_spec["name"], attr_spec.is_multi, decoded) + try: + if attr_spec["type"] == 'nest': + subdict = self._decode(NlAttrs(attr.raw), attr_spec['nested-attributes'], search_attrs) + decoded = subdict + elif attr_spec["type"] == 'string': + decoded = attr.as_strz() + elif attr_spec["type"] == 'binary': + decoded = self._decode_binary(attr, attr_spec) + elif attr_spec["type"] == 'flag': + decoded = True + elif attr_spec.is_auto_scalar: + decoded = attr.as_auto_scalar(attr_spec['type'], attr_spec.byte_order) + elif attr_spec["type"] in NlAttr.type_formats: + decoded = attr.as_scalar(attr_spec['type'], attr_spec.byte_order) + if 'enum' in attr_spec: + decoded = self._decode_enum(decoded, attr_spec) + elif attr_spec.display_hint: + decoded = self._formatted_string(decoded, attr_spec.display_hint) + elif attr_spec["type"] == 'indexed-array': + decoded = self._decode_array_attr(attr, attr_spec) + elif attr_spec["type"] == 'bitfield32': + value, selector = struct.unpack("II", attr.raw) + if 'enum' in attr_spec: + value = self._decode_enum(value, attr_spec) + selector = self._decode_enum(selector, attr_spec) + decoded = {"value": value, "selector": selector} + elif attr_spec["type"] == 'sub-message': + decoded = self._decode_sub_msg(attr, attr_spec, search_attrs) + elif attr_spec["type"] == 'nest-type-value': + decoded = self._decode_nest_type_value(attr, attr_spec) + else: + if not self.process_unknown: + raise Exception(f'Unknown {attr_spec["type"]} with name {attr_spec["name"]}') + decoded = self._decode_unknown(attr) + + self._rsp_add(rsp, attr_spec["name"], attr_spec.is_multi, decoded) + except: + print(f"Error decoding '{attr_spec.name}' from '{space}'") + raise return rsp -- 2.51.0 From 6ffdbb93a59c60ee18bebd9acdbbca91e6bf0e64 Mon Sep 17 00:00:00 2001 From: Jakub Kicinski Date: Mon, 6 Jan 2025 18:28:20 -0800 Subject: [PATCH 14/16] netlink: specs: rt_link: decode ip6tnl, vti and vti6 link attrs Some of our tests load vti and ip6tnl so not being able to decode the link attrs gets in the way of using Python YNL for testing. Decode link attributes for ip6tnl, vti and vti6. ip6tnl uses IFLA_IPTUN_FLAGS as u32, while ipv4 and sit expect a u16 attribute, so we have a (first?) subset type override... Reviewed-by: Donald Hunter Acked-by: Stanislav Fomichev Link: https://patch.msgid.link/20250107022820.2087101-4-kuba@kernel.org Signed-off-by: Jakub Kicinski --- Documentation/netlink/specs/rt_link.yaml | 87 ++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/Documentation/netlink/specs/rt_link.yaml b/Documentation/netlink/specs/rt_link.yaml index 2f62d51c26cd..0d492500c7e5 100644 --- a/Documentation/netlink/specs/rt_link.yaml +++ b/Documentation/netlink/specs/rt_link.yaml @@ -1825,6 +1825,48 @@ attribute-sets: - name: erspan-hwid type: u16 + - + name: linkinfo-vti-attrs + name-prefix: ifla-vti- + attributes: + - + name: link + type: u32 + - + name: ikey + type: u32 + - + name: okey + type: u32 + - + name: local + type: binary + display-hint: ipv4 + - + name: remote + type: binary + display-hint: ipv4 + - + name: fwmark + type: u32 + - + name: linkinfo-vti6-attrs + subset-of: linkinfo-vti-attrs + attributes: + - + name: link + - + name: ikey + - + name: okey + - + name: local + display-hint: ipv6 + - + name: remote + display-hint: ipv6 + - + name: fwmark - name: linkinfo-geneve-attrs name-prefix: ifla-geneve- @@ -1941,6 +1983,42 @@ attribute-sets: - name: fwmark type: u32 + - + name: linkinfo-ip6tnl-attrs + subset-of: linkinfo-iptun-attrs + attributes: + - + name: link + - + name: local + display-hint: ipv6 + - + name: remote + display-hint: ipv6 + - + name: ttl + - + name: encap-limit + - + name: flowinfo + - + name: flags + # ip6tnl unlike ipip and sit has 32b flags + type: u32 + - + name: proto + - + name: encap-type + - + name: encap-flags + - + name: encap-sport + - + name: encap-dport + - + name: collect-metadata + - + name: fwmark - name: linkinfo-tun-attrs name-prefix: ifla-tun- @@ -2201,6 +2279,9 @@ sub-messages: - value: ipip attribute-set: linkinfo-iptun-attrs + - + value: ip6tnl + attribute-set: linkinfo-ip6tnl-attrs - value: sit attribute-set: linkinfo-iptun-attrs @@ -2213,6 +2294,12 @@ sub-messages: - value: vrf attribute-set: linkinfo-vrf-attrs + - + value: vti + attribute-set: linkinfo-vti-attrs + - + value: vti6 + attribute-set: linkinfo-vti6-attrs - value: netkit attribute-set: linkinfo-netkit-attrs -- 2.51.0 From f70b864ccc84a024c765306e95e8e390834c263d Mon Sep 17 00:00:00 2001 From: Sriram Yagnaraman Date: Mon, 6 Jan 2025 14:19:09 -0800 Subject: [PATCH 15/16] igb: Remove static qualifiers Remove static qualifiers on the following functions to be able to call from XSK specific file that is added in the later patches: - igb_xdp_tx_queue_mapping() - igb_xdp_ring_update_tail() - igb_clean_tx_ring() - igb_clean_rx_ring() - igb_xdp_xmit_back() - igb_process_skb_fields() While at it, inline igb_xdp_tx_queue_mapping() and igb_xdp_ring_update_tail(). These functions are small enough and used in XDP hot paths. Signed-off-by: Sriram Yagnaraman [Kurt: Split patches, inline small XDP functions] Signed-off-by: Kurt Kanzenbach Acked-by: Maciej Fijalkowski Tested-by: George Kuruvinakunnel Signed-off-by: Tony Nguyen Link: https://patch.msgid.link/20250106221929.956999-2-anthony.l.nguyen@intel.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/intel/igb/igb.h | 29 ++++++++++++++++++ drivers/net/ethernet/intel/igb/igb_main.c | 37 ++++------------------- 2 files changed, 35 insertions(+), 31 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 3c2dc7bdebb5..1bfe703e73d9 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -18,6 +18,7 @@ #include #include #include +#include #include @@ -731,12 +732,18 @@ int igb_setup_tx_resources(struct igb_ring *); int igb_setup_rx_resources(struct igb_ring *); void igb_free_tx_resources(struct igb_ring *); void igb_free_rx_resources(struct igb_ring *); +void igb_clean_tx_ring(struct igb_ring *tx_ring); +void igb_clean_rx_ring(struct igb_ring *rx_ring); void igb_configure_tx_ring(struct igb_adapter *, struct igb_ring *); void igb_configure_rx_ring(struct igb_adapter *, struct igb_ring *); void igb_setup_tctl(struct igb_adapter *); void igb_setup_rctl(struct igb_adapter *); void igb_setup_srrctl(struct igb_adapter *, struct igb_ring *); netdev_tx_t igb_xmit_frame_ring(struct sk_buff *, struct igb_ring *); +int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp); +void igb_process_skb_fields(struct igb_ring *rx_ring, + union e1000_adv_rx_desc *rx_desc, + struct sk_buff *skb); void igb_alloc_rx_buffers(struct igb_ring *, u16); void igb_update_stats(struct igb_adapter *); bool igb_has_link(struct igb_adapter *adapter); @@ -797,6 +804,28 @@ static inline struct netdev_queue *txring_txq(const struct igb_ring *tx_ring) return netdev_get_tx_queue(tx_ring->netdev, tx_ring->queue_index); } +/* This function assumes __netif_tx_lock is held by the caller. */ +static inline void igb_xdp_ring_update_tail(struct igb_ring *ring) +{ + lockdep_assert_held(&txring_txq(ring)->_xmit_lock); + + /* Force memory writes to complete before letting h/w know there + * are new descriptors to fetch. + */ + wmb(); + writel(ring->next_to_use, ring->tail); +} + +static inline struct igb_ring *igb_xdp_tx_queue_mapping(struct igb_adapter *adapter) +{ + unsigned int r_idx = smp_processor_id(); + + if (r_idx >= adapter->num_tx_queues) + r_idx = r_idx % adapter->num_tx_queues; + + return adapter->tx_ring[r_idx]; +} + int igb_add_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input); int igb_erase_filter(struct igb_adapter *adapter, diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 288a4bb2683a..77a6d1470f8b 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -33,7 +33,6 @@ #include #include #include -#include #ifdef CONFIG_IGB_DCA #include #endif @@ -116,8 +115,6 @@ static void igb_configure_tx(struct igb_adapter *); static void igb_configure_rx(struct igb_adapter *); static void igb_clean_all_tx_rings(struct igb_adapter *); static void igb_clean_all_rx_rings(struct igb_adapter *); -static void igb_clean_tx_ring(struct igb_ring *); -static void igb_clean_rx_ring(struct igb_ring *); static void igb_set_rx_mode(struct net_device *); static void igb_update_phy_info(struct timer_list *); static void igb_watchdog(struct timer_list *); @@ -2919,29 +2916,7 @@ static int igb_xdp(struct net_device *dev, struct netdev_bpf *xdp) } } -/* This function assumes __netif_tx_lock is held by the caller. */ -static void igb_xdp_ring_update_tail(struct igb_ring *ring) -{ - lockdep_assert_held(&txring_txq(ring)->_xmit_lock); - - /* Force memory writes to complete before letting h/w know there - * are new descriptors to fetch. - */ - wmb(); - writel(ring->next_to_use, ring->tail); -} - -static struct igb_ring *igb_xdp_tx_queue_mapping(struct igb_adapter *adapter) -{ - unsigned int r_idx = smp_processor_id(); - - if (r_idx >= adapter->num_tx_queues) - r_idx = r_idx % adapter->num_tx_queues; - - return adapter->tx_ring[r_idx]; -} - -static int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp) +int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp) { struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp); int cpu = smp_processor_id(); @@ -4888,7 +4863,7 @@ static void igb_free_all_tx_resources(struct igb_adapter *adapter) * igb_clean_tx_ring - Free Tx Buffers * @tx_ring: ring to be cleaned **/ -static void igb_clean_tx_ring(struct igb_ring *tx_ring) +void igb_clean_tx_ring(struct igb_ring *tx_ring) { u16 i = tx_ring->next_to_clean; struct igb_tx_buffer *tx_buffer = &tx_ring->tx_buffer_info[i]; @@ -5007,7 +4982,7 @@ static void igb_free_all_rx_resources(struct igb_adapter *adapter) * igb_clean_rx_ring - Free Rx Buffers per Queue * @rx_ring: ring to free buffers from **/ -static void igb_clean_rx_ring(struct igb_ring *rx_ring) +void igb_clean_rx_ring(struct igb_ring *rx_ring) { u16 i = rx_ring->next_to_clean; @@ -8786,9 +8761,9 @@ static bool igb_cleanup_headers(struct igb_ring *rx_ring, * order to populate the hash, checksum, VLAN, timestamp, protocol, and * other fields within the skb. **/ -static void igb_process_skb_fields(struct igb_ring *rx_ring, - union e1000_adv_rx_desc *rx_desc, - struct sk_buff *skb) +void igb_process_skb_fields(struct igb_ring *rx_ring, + union e1000_adv_rx_desc *rx_desc, + struct sk_buff *skb) { struct net_device *dev = rx_ring->netdev; -- 2.51.0 From 6dc75fc230eceedada4adf8c2857636a9dbd00a8 Mon Sep 17 00:00:00 2001 From: Sriram Yagnaraman Date: Mon, 6 Jan 2025 14:19:10 -0800 Subject: [PATCH 16/16] igb: Introduce igb_xdp_is_enabled() Introduce igb_xdp_is_enabled() to check if an XDP program is assigned to the device. Use that wherever xdp_prog is read and evaluated. Signed-off-by: Sriram Yagnaraman [Kurt: Split patches and use READ_ONCE()] Signed-off-by: Kurt Kanzenbach Acked-by: Maciej Fijalkowski Tested-by: George Kuruvinakunnel Signed-off-by: Tony Nguyen Link: https://patch.msgid.link/20250106221929.956999-3-anthony.l.nguyen@intel.com Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/intel/igb/igb.h | 5 +++++ drivers/net/ethernet/intel/igb/igb_main.c | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h index 1bfe703e73d9..6e2b61ecff68 100644 --- a/drivers/net/ethernet/intel/igb/igb.h +++ b/drivers/net/ethernet/intel/igb/igb.h @@ -826,6 +826,11 @@ static inline struct igb_ring *igb_xdp_tx_queue_mapping(struct igb_adapter *adap return adapter->tx_ring[r_idx]; } +static inline bool igb_xdp_is_enabled(struct igb_adapter *adapter) +{ + return !!READ_ONCE(adapter->xdp_prog); +} + int igb_add_filter(struct igb_adapter *adapter, struct igb_nfc_filter *input); int igb_erase_filter(struct igb_adapter *adapter, diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index 77a6d1470f8b..fef58c55eb43 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -2930,7 +2930,8 @@ int igb_xdp_xmit_back(struct igb_adapter *adapter, struct xdp_buff *xdp) /* During program transitions its possible adapter->xdp_prog is assigned * but ring has not been configured yet. In this case simply abort xmit. */ - tx_ring = adapter->xdp_prog ? igb_xdp_tx_queue_mapping(adapter) : NULL; + tx_ring = igb_xdp_is_enabled(adapter) ? + igb_xdp_tx_queue_mapping(adapter) : NULL; if (unlikely(!tx_ring)) return IGB_XDP_CONSUMED; @@ -2963,7 +2964,8 @@ static int igb_xdp_xmit(struct net_device *dev, int n, /* During program transitions its possible adapter->xdp_prog is assigned * but ring has not been configured yet. In this case simply abort xmit. */ - tx_ring = adapter->xdp_prog ? igb_xdp_tx_queue_mapping(adapter) : NULL; + tx_ring = igb_xdp_is_enabled(adapter) ? + igb_xdp_tx_queue_mapping(adapter) : NULL; if (unlikely(!tx_ring)) return -ENXIO; @@ -6597,7 +6599,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu) struct igb_adapter *adapter = netdev_priv(netdev); int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD; - if (adapter->xdp_prog) { + if (igb_xdp_is_enabled(adapter)) { int i; for (i = 0; i < adapter->num_rx_queues; i++) { -- 2.51.0