net: ndo_fdb_add/del: Have drivers report whether they notified
Subsystems (10)
Petr Machata
maintainerv3 cover net: ndo_fdb_add/del: Have drivers report whether they notified
Currently when FDB entries are added to or deleted from a VXLAN netdevice, the VXLAN driver emits one notification, including the VXLAN-specific attributes. The core however always sends a notification as well, a generic one. Thus two notifications are unnecessarily sent for these operations. A similar situation comes up with bridge driver, which also emits notifications on its own.
# ip link add name vx type vxlan id 1000 dstport 4789 # bridge monitor fdb & [1] 1981693 # bridge fdb add de:ad:be:ef:13:37 dev vx self dst 192.0.2.1 de:ad:be:ef:13:37 dev vx dst 192.0.2.1 self permanent de:ad:be:ef:13:37 dev vx self permanent
In order to prevent this duplicity, add a parameter, bool *notified, to ndo_fdb_add and ndo_fdb_del. The flag is primed to false, and if the callee sends a notification on its own, it sets the flag to true, thus informing the core that it should not generate another notification.
Patches #1 to #2 are concerned with the above.
In the remaining patches, #3 to #7, add a selftest. This takes place across several patches. Many of the helpers we would like to use for the test are in forwarding/lib.sh, whereas net/ is a more suitable place for the test, so the libraries need to be massaged a bit first.
v3:
- v1 and v2 differed from this version mainly because they outright shifted the responsibility for notifying to the callee.
- Both substance patches were reworked, patch #1 was dropped. Selftest patches stayed intact.
v2:
- Patches #2, #3:
- Fix qlcnic build
Petr Machata (7):
ndo_fdb_add: Add a parameter to report whether notification was sent
ndo_fdb_del: Add a parameter to report whether notification was sent
selftests: net: lib: Move logging from forwarding/lib.sh here
selftests: net: lib: Move tests_run from forwarding/lib.sh here
selftests: net: lib: Move checks from forwarding/lib.sh here
selftests: net: lib: Add kill_process
selftests: net: fdb_notify: Add a test for FDB notifications
drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +- drivers/net/ethernet/intel/ice/ice_main.c | 8 +- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/mscc/ocelot_net.c | 4 +- .../net/ethernet/qlogic/qlcnic/qlcnic_main.c | 4 +- drivers/net/macvlan.c | 4 +- drivers/net/vxlan/vxlan_core.c | 10 +- include/linux/netdevice.h | 14 +- net/bridge/br_fdb.c | 27 ++- net/bridge/br_private.h | 4 +- net/core/rtnetlink.c | 20 +- .../drivers/net/mlxsw/devlink_trap.sh | 2 +- .../net/mlxsw/devlink_trap_l3_drops.sh | 4 +- .../net/mlxsw/devlink_trap_l3_exceptions.sh | 12 +- .../net/mlxsw/devlink_trap_tunnel_ipip.sh | 4 +- .../net/mlxsw/devlink_trap_tunnel_ipip6.sh | 4 +- .../net/mlxsw/devlink_trap_tunnel_vxlan.sh | 4 +- .../mlxsw/devlink_trap_tunnel_vxlan_ipv6.sh | 4 +- .../selftests/drivers/net/mlxsw/tc_sample.sh | 4 +- .../net/netdevsim/fib_notifications.sh | 6 +- tools/testing/selftests/net/Makefile | 2 +- .../selftests/net/drop_monitor_tests.sh | 2 +- tools/testing/selftests/net/fdb_notify.sh | 95 ++++++++ tools/testing/selftests/net/fib_tests.sh | 8 +- .../selftests/net/forwarding/devlink_lib.sh | 2 +- tools/testing/selftests/net/forwarding/lib.sh | 199 +--------------- .../selftests/net/forwarding/tc_police.sh | 8 +- tools/testing/selftests/net/lib.sh | 223 ++++++++++++++++++ 29 files changed, 418 insertions(+), 267 deletions(-) create mode 100755 tools/testing/selftests/net/fdb_notify.sh
Signature · 1 line
2.45.0
Petr Machata
maintainerPATCH v3 1/7 ndo_fdb_add: Add a parameter to report whether notification was sent
Currently when FDB entries are added to or deleted from a VXLAN netdevice, the VXLAN driver emits one notification, including the VXLAN-specific attributes. The core however always sends a notification as well, a generic one. Thus two notifications are unnecessarily sent for these operations. A similar situation comes up with bridge driver, which also emits notifications on its own:
# ip link add name vx type vxlan id 1000 dstport 4789 # bridge monitor fdb & [1] 1981693 # bridge fdb add de:ad:be:ef:13:37 dev vx self dst 192.0.2.1 de:ad:be:ef:13:37 dev vx dst 192.0.2.1 self permanent de:ad:be:ef:13:37 dev vx self permanent
In order to prevent this duplicity, add a paremeter to ndo_fdb_add, bool *notified. The flag is primed to false, and if the callee sends a notification on its own, it sets it to true, thus informing the core that it should not generate another notification.
---
Notes:
CC: Simon Horman <[email protected]>
CC: Przemek Kitszel <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Manish Chopra <[email protected]>
CC: [email protected]
CC: Kuniyuki Iwashima <[email protected]>
CC: Andrew Lunn <[email protected]>
CC: Nikolay Aleksandrov <[email protected]>
CC: [email protected]
drivers/net/ethernet/intel/i40e/i40e_main.c | 3 ++- drivers/net/ethernet/intel/ice/ice_main.c | 4 +++- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/mscc/ocelot_net.c | 2 +- drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +- drivers/net/macvlan.c | 2 +- drivers/net/vxlan/vxlan_core.c | 5 ++++- include/linux/netdevice.h | 5 ++++- net/bridge/br_fdb.c | 12 +++++++----- net/bridge/br_private.h | 2 +- net/core/rtnetlink.c | 9 ++++++--- 12 files changed, 32 insertions(+), 18 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_main.c +2 -1
@@ -13095,12 +13095,13 @@ static int i40e_get_phys_port_id(struct net_device *netdev,* @addr: the MAC address entry being added * @vid: VLAN ID * @flags: instructions from stack about fdb operation +* @notified: whether notification was emitted * @extack: netlink extended ack, unused currently */ static int i40e_ndo_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -u16 flags, +u16 flags,bool *notified,struct netlink_ext_ack *extack) {struct i40e_netdev_priv *np =netdev_priv (dev);
drivers/net/ethernet/intel/ice/ice_main.c +3 -1
@@ -6125,12 +6125,14 @@ ice_set_tx_maxrate(struct net_device *netdev, int queue_index, u32 maxrate)* @addr: the MAC address entry being added * @vid: VLAN ID * @flags: instructions from stack about fdb operation +* @notified: whether notification was emitted * @extack: netlink extended ack */ static int ice_fdb_add (struct ndmsg *ndm,struct nlattr __always_unused *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -u16 flags,struct netlink_ext_ack __always_unused *extack) +u16 flags,bool *notified, +struct netlink_ext_ack __always_unused *extack) {int err;
drivers/net/ethernet/intel/igb/igb_main.c +1 -1
@@ -2486,7 +2486,7 @@ static int igb_set_features(struct net_device *netdev,static int igb_ndo_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -u16 flags, +u16 flags,bool *notified,struct netlink_ext_ack *extack) {/* guarantee we can provide a unique filter for the unicast address */
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +1 -1
@@ -9954,7 +9954,7 @@ static int ixgbe_set_features(struct net_device *netdev,static int ixgbe_ndo_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -u16 flags, +u16 flags,bool *notified,struct netlink_ext_ack *extack) {/* guarantee we can provide a unique filter for the unicast address */
drivers/net/ethernet/mscc/ocelot_net.c +1 -1
@@ -730,7 +730,7 @@ static void ocelot_get_stats64(struct net_device *dev,static int ocelot_port_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr, -u16 vid,u16 flags, +u16 vid,u16 flags,bool *notified,struct netlink_ext_ack *extack) {struct ocelot_port_private *priv =netdev_priv (dev);
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c +1 -1
@@ -394,7 +394,7 @@ static int qlcnic_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],static int qlcnic_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *netdev,const unsigned char *addr,u16 vid,u16 flags, -struct netlink_ext_ack *extack) +bool *notified,struct netlink_ext_ack *extack) {struct qlcnic_adapter *adapter =netdev_priv (netdev);int err =0 ;
drivers/net/macvlan.c +1 -1
@@ -1024,7 +1024,7 @@ static int macvlan_vlan_rx_kill_vid(struct net_device *dev,static int macvlan_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -u16 flags, +u16 flags,bool *notified,struct netlink_ext_ack *extack) {struct macvlan_dev *vlan =netdev_priv (dev);
drivers/net/vxlan/vxlan_core.c +4 -1
@@ -1244,7 +1244,7 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,static int vxlan_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid,u16 flags, -struct netlink_ext_ack *extack) +bool *notified,struct netlink_ext_ack *extack) {struct vxlan_dev *vxlan =netdev_priv (dev);/* struct net *net = dev_net(vxlan->dev); */
@@ -1280,6 +1280,9 @@ static int vxlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[], nhid, true, extack);spin_unlock_bh (&vxlan->hash_lock[hash_index]); +if (!err) + *notified = true; +return err; }
include/linux/netdevice.h +4 -1
@@ -1246,8 +1246,10 @@ struct netdev_net_notifier {* int (*ndo_fdb_add)(struct ndmsg *ndm, struct nlattr *tb[], * struct net_device *dev, * const unsigned char *addr, u16 vid, u16 flags, -* struct netlink_ext_ack *extack); +* bool *notified, struct netlink_ext_ack *extack); * Adds an FDB entry to dev for addr. +* Callee shall set *notified to true if it sent any appropriate +* notification(s). Otherwise core will send a generic one. * int (*ndo_fdb_del)(struct ndmsg *ndm, struct nlattr *tb[], * struct net_device *dev, * const unsigned char *addr, u16 vid)
@@ -1523,6 +1525,7 @@ struct net_device_ops {const unsigned char *addr, u16 vid, u16 flags, + bool *notified, struct netlink_ext_ack *extack); int (*ndo_fdb_del)(struct ndmsg *ndm, struct nlattr *tb[],
net/bridge/br_fdb.c +7 -5
@@ -1152,7 +1152,7 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,static int __br_fdb_add (struct ndmsg *ndm,struct net_bridge *br,struct net_bridge_port *p,const unsigned char *addr,u16 nlh_flags,u16 vid,struct nlattr *nfea_tb[], -struct netlink_ext_ack *extack) +bool *notified,struct netlink_ext_ack *extack) {int err =0 ;
@@ -1183,6 +1183,8 @@ static int __br_fdb_add(struct ndmsg *ndm, struct net_bridge *br,spin_unlock_bh (&br->hash_lock); } +if (!err) + *notified = true;return err; }
@@ -1195,7 +1197,7 @@ static const struct nla_policy br_nda_fdb_pol[NFEA_MAX + 1] = {int br_fdb_add (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid,u16 nlh_flags, -struct netlink_ext_ack *extack) +bool *notified,struct netlink_ext_ack *extack) {struct nlattr *nfea_tb[NFEA_MAX +1 ], *attr;struct net_bridge_vlan_group *vg;
@@ -1258,10 +1260,10 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],/* VID was specified, so use it. */ err =__br_fdb_add (ndm, br, p, addr, nlh_flags, vid, nfea_tb, - extack); + notified, extack); }else { err =__br_fdb_add (ndm, br, p, addr, nlh_flags,0 , nfea_tb, - extack); + notified, extack);if (err || !vg || !vg->num_vlans) goto out;
@@ -1273,7 +1275,7 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],if (!br_vlan_should_use (v))continue ; err =__br_fdb_add (ndm, br, p, addr, nlh_flags, v->vid, - nfea_tb, extack); + nfea_tb, notified, extack);if (err) goto out; }
net/bridge/br_private.h +1 -1
@@ -858,7 +858,7 @@ int br_fdb_delete_bulk(struct nlmsghdr *nlh, struct net_device *dev, struct netlink_ext_ack *extack);int br_fdb_add (struct ndmsg *nlh,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid,u16 nlh_flags, -struct netlink_ext_ack *extack); +bool *notified,struct netlink_ext_ack *extack);int br_fdb_dump (struct sk_buff *skb,struct netlink_callback *cb,struct net_device *dev,struct net_device *fdev,int *idx); int br_fdb_get(struct sk_buff *skb, struct nlattr *tb[], struct net_device *dev,
net/core/rtnetlink.c +6 -3
@@ -4465,9 +4465,10 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh, netif_is_bridge_port(dev)) {struct net_device *br_dev =netdev_master_upper_dev_get (dev);const struct net_device_ops *ops = br_dev->netdev_ops; +bool notified = false; err = ops->ndo_fdb_add (ndm, tb, dev, addr, vid, - nlh->nlmsg_flags, extack); + nlh->nlmsg_flags, ¬ified, extack);if (err) goto out;else
@@ -4476,16 +4477,18 @@ static int rtnl_fdb_add(struct sk_buff *skb, struct nlmsghdr *nlh,/* Embedded bridge, macvlan, and any other device support */ if ((ndm->ndm_flags &NTF_SELF )) { +bool notified = false; +if (dev->netdev_ops->ndo_fdb_add) err = dev->netdev_ops->ndo_fdb_add (ndm, tb, dev, addr, vid, nlh->nlmsg_flags, - extack); + ¬ified, extack);else err =ndo_dflt_fdb_add (ndm, tb, dev, addr, vid, nlh->nlmsg_flags); -if (!err) { +if (!err && !notified) {rtnl_fdb_notify (dev, addr, vid,RTM_NEWNEIGH , ndm->ndm_state); ndm->ndm_flags &= ~NTF_SELF ;
Signature · 1 line
2.45.0
Re: [PATCH net-next v3 1/7] ndo_fdb_add: Add a parameter to report whether notification was sent
Quoted 35 lines · Petr Machata
Currently when FDB entries are added to or deleted from a VXLAN netdevice, the VXLAN driver emits one notification, including the VXLAN-specific attributes. The core however always sends a notification as well, a generic one. Thus two notifications are unnecessarily sent for these operations. A similar situation comes up with bridge driver, which also emits notifications on its own:
# ip link add name vx type vxlan id 1000 dstport 4789 # bridge monitor fdb & [1] 1981693 # bridge fdb add de:ad:be:ef:13:37 dev vx self dst 192.0.2.1 de:ad:be:ef:13:37 dev vx dst 192.0.2.1 self permanent de:ad:be:ef:13:37 dev vx self permanentIn order to prevent this duplicity, add a paremeter to
ndo_fdb_add, bool *notified. The flag is primed to false, and if the callee sends a notification on its own, it sets it to true, thus informing the core that it should not generate another notification.Signed-off-by Petr Machata <[email protected]>Reviewed-by Amit Cohen <[email protected]>---
Notes:
CC: Simon Horman <[email protected]>
CC: Przemek Kitszel <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Manish Chopra <[email protected]>
CC: [email protected]
CC: Kuniyuki Iwashima <[email protected]>
CC: Andrew Lunn <[email protected]>
CC: Nikolay Aleksandrov <[email protected]>
CC: [email protected]
Quoted 14 lines
drivers/net/ethernet/intel/i40e/i40e_main.c | 3 ++- drivers/net/ethernet/intel/ice/ice_main.c | 4 +++- drivers/net/ethernet/intel/igb/igb_main.c | 2 +- drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 2 +- drivers/net/ethernet/mscc/ocelot_net.c | 2 +- drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +- drivers/net/macvlan.c | 2 +- drivers/net/vxlan/vxlan_core.c | 5 ++++- include/linux/netdevice.h | 5 ++++- net/bridge/br_fdb.c | 12 +++++++----- net/bridge/br_private.h | 2 +- net/core/rtnetlink.c | 9 ++++++--- 12 files changed, 32 insertions(+), 18 deletions(-)
LGTM,
Petr Machata
maintainerPATCH v3 2/7 ndo_fdb_del: Add a parameter to report whether notification was sent
In reply to Petr MachataIn a similar fashion to ndo_fdb_add, which was covered in the previous patch, add the bool *notified argument to ndo_fdb_del. Callees that send a notification on their own set the flag to true.
---
Notes:
CC: Przemek Kitszel <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Manish Chopra <[email protected]>
CC: [email protected]
CC: Kuniyuki Iwashima <[email protected]>
CC: Andrew Lunn <[email protected]>
CC: Nikolay Aleksandrov <[email protected]>
CC: [email protected]
drivers/net/ethernet/intel/ice/ice_main.c | 4 +++- drivers/net/ethernet/mscc/ocelot_net.c | 2 +- drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +- drivers/net/macvlan.c | 2 +- drivers/net/vxlan/vxlan_core.c | 5 ++++- include/linux/netdevice.h | 9 +++++++-- net/bridge/br_fdb.c | 15 ++++++++------- net/bridge/br_private.h | 2 +- net/core/rtnetlink.c | 11 ++++++++--- 9 files changed, 34 insertions(+), 18 deletions(-)
drivers/net/ethernet/intel/ice/ice_main.c +3 -1
@@ -6166,12 +6166,14 @@ ice_fdb_add(struct ndmsg *ndm, struct nlattr __always_unused *tb[],* @dev: the net device pointer * @addr: the MAC address entry being added * @vid: VLAN ID +* @notified: whether notification was emitted * @extack: netlink extended ack */ static int ice_fdb_del (struct ndmsg *ndm,__always_unused struct nlattr *tb[],struct net_device *dev,const unsigned char *addr, -__always_unused u16 vid,struct netlink_ext_ack *extack) +__always_unused u16 vid,bool *notified, +struct netlink_ext_ack *extack) {int err;
drivers/net/ethernet/mscc/ocelot_net.c +1 -1
@@ -744,7 +744,7 @@ static int ocelot_port_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],static int ocelot_port_fdb_del (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -struct netlink_ext_ack *extack) +bool *notified,struct netlink_ext_ack *extack) {struct ocelot_port_private *priv =netdev_priv (dev);struct ocelot_port *ocelot_port = &priv->port;
drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c +1 -1
@@ -367,7 +367,7 @@ static int qlcnic_set_mac(struct net_device *netdev, void *p)static int qlcnic_fdb_del (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *netdev, -const unsigned char *addr,u16 vid, +const unsigned char *addr,u16 vid,bool *notified,struct netlink_ext_ack *extack) {struct qlcnic_adapter *adapter =netdev_priv (netdev);
drivers/net/macvlan.c +1 -1
@@ -1049,7 +1049,7 @@ static int macvlan_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],static int macvlan_fdb_del (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev, -const unsigned char *addr,u16 vid, +const unsigned char *addr,u16 vid,bool *notified,struct netlink_ext_ack *extack) {struct macvlan_dev *vlan =netdev_priv (dev);
drivers/net/vxlan/vxlan_core.c +4 -1
@@ -1322,7 +1322,7 @@ int __vxlan_fdb_delete(struct vxlan_dev *vxlan,/* Delete entry (via netlink) */ static int vxlan_fdb_delete (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev, -const unsigned char *addr,u16 vid, +const unsigned char *addr,u16 vid,bool *notified,struct netlink_ext_ack *extack) {struct vxlan_dev *vxlan =netdev_priv (dev);
@@ -1344,6 +1344,9 @@ static int vxlan_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], true);spin_unlock_bh (&vxlan->hash_lock[hash_index]); +if (!err) + *notified = true; +return err; }
include/linux/netdevice.h +7 -2
@@ -1252,8 +1252,11 @@ struct netdev_net_notifier {* notification(s). Otherwise core will send a generic one. * int (*ndo_fdb_del)(struct ndmsg *ndm, struct nlattr *tb[], * struct net_device *dev, -* const unsigned char *addr, u16 vid) +* const unsigned char *addr, u16 vid +* bool *notified, struct netlink_ext_ack *extack); * Deletes the FDB entry from dev corresponding to addr. +* Callee shall set *notified to true if it sent any appropriate +* notification(s). Otherwise core will send a generic one. * int (*ndo_fdb_del_bulk)(struct nlmsghdr *nlh, struct net_device *dev, * struct netlink_ext_ack *extack); * int (*ndo_fdb_dump)(struct sk_buff *skb, struct netlink_callback *cb,
@@ -1531,7 +1534,9 @@ struct net_device_ops {struct nlattr *tb[], struct net_device *dev, const unsigned char *addr, - u16 vid, struct netlink_ext_ack *extack); + u16 vid, + bool *notified, + struct netlink_ext_ack *extack);int (*ndo_fdb_del_bulk)(struct nlmsghdr *nlh,struct net_device *dev,struct netlink_ext_ack *extack);
net/bridge/br_fdb.c +8 -7
@@ -1287,7 +1287,7 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],static int fdb_delete_by_addr_and_port (struct net_bridge *br,const struct net_bridge_port *p, -const u8 *addr,u16 vlan) +const u8 *addr,u16 vlan,bool *notified) {struct net_bridge_fdb_entry *fdb;
@@ -1296,18 +1296,19 @@ static int fdb_delete_by_addr_and_port(struct net_bridge *br,return -ENOENT ;fdb_delete (br, fdb, true); + *notified = true;return 0 ; }static int __br_fdb_delete (struct net_bridge *br,const struct net_bridge_port *p, -const unsigned char *addr,u16 vid) +const unsigned char *addr,u16 vid,bool *notified) {int err;spin_lock_bh (&br->hash_lock); - err =fdb_delete_by_addr_and_port (br, p, addr, vid); + err =fdb_delete_by_addr_and_port (br, p, addr, vid, notified);spin_unlock_bh (&br->hash_lock);return err;
@@ -1316,7 +1317,7 @@ static int __br_fdb_delete(struct net_bridge *br,/* Remove neighbor entry with RTM_DELNEIGH */ int br_fdb_delete (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev, -const unsigned char *addr,u16 vid, +const unsigned char *addr,u16 vid,bool *notified,struct netlink_ext_ack *extack) {struct net_bridge_vlan_group *vg;
@@ -1339,19 +1340,19 @@ int br_fdb_delete(struct ndmsg *ndm, struct nlattr *tb[], }if (vid) { - err =__br_fdb_delete (br, p, addr, vid); + err =__br_fdb_delete (br, p, addr, vid, notified); }else {struct net_bridge_vlan *v; err = -ENOENT ; - err &=__br_fdb_delete (br, p, addr,0 ); + err &=__br_fdb_delete (br, p, addr,0 , notified);if (!vg || !vg->num_vlans)return err;list_for_each_entry (v, &vg->vlan_list, vlist) {if (!br_vlan_should_use (v))continue ; - err &=__br_fdb_delete (br, p, addr, v->vid); + err &=__br_fdb_delete (br, p, addr, v->vid, notified); } }
net/bridge/br_private.h +1 -1
@@ -853,7 +853,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,int br_fdb_delete (struct ndmsg *ndm,struct nlattr *tb[],struct net_device *dev,const unsigned char *addr,u16 vid, -struct netlink_ext_ack *extack); +bool *notified,struct netlink_ext_ack *extack);int br_fdb_delete_bulk (struct nlmsghdr *nlh,struct net_device *dev,struct netlink_ext_ack *extack); int br_fdb_add(struct ndmsg *nlh, struct nlattr *tb[], struct net_device *dev,
net/core/rtnetlink.c +8 -3
@@ -4588,11 +4588,13 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,if ((!ndm->ndm_flags || ndm->ndm_flags &NTF_MASTER ) &&netif_is_bridge_port (dev)) {struct net_device *br_dev =netdev_master_upper_dev_get (dev); +bool notified = false; ops = br_dev->netdev_ops;if (!del_bulk) {if (ops->ndo_fdb_del) - err = ops->ndo_fdb_del (ndm, tb, dev, addr, vid, extack); + err = ops->ndo_fdb_del (ndm, tb, dev, addr, vid, + ¬ified, extack); }else {if (ops->ndo_fdb_del_bulk) err = ops->ndo_fdb_del_bulk (nlh, dev, extack);
@@ -4606,10 +4608,13 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,/* Embedded bridge, macvlan, and any other device support */ if (ndm->ndm_flags &NTF_SELF ) { +bool notified = false; + ops = dev->netdev_ops;if (!del_bulk) {if (ops->ndo_fdb_del) - err = ops->ndo_fdb_del (ndm, tb, dev, addr, vid, extack); + err = ops->ndo_fdb_del (ndm, tb, dev, addr, vid, + ¬ified, extack);else err =ndo_dflt_fdb_del (ndm, tb, dev, addr, vid); }else {
@@ -4620,7 +4625,7 @@ static int rtnl_fdb_del(struct sk_buff *skb, struct nlmsghdr *nlh, }if (!err) { -if (!del_bulk) +if (!del_bulk && !notified)rtnl_fdb_notify (dev, addr, vid,RTM_DELNEIGH , ndm->ndm_state); ndm->ndm_flags &= ~NTF_SELF ;
Signature · 1 line
2.45.0
Re: [PATCH net-next v3 2/7] ndo_fdb_del: Add a parameter to report whether notification was sent
Quoted 19 lines · Petr Machata
In a similar fashion to
ndo_fdb_add, which was covered in the previous patch, add the bool *notified argument tondo_fdb_del. Callees that send a notification on their own set the flag to true.Signed-off-by Petr Machata <[email protected]>Reviewed-by Amit Cohen <[email protected]>---
Notes:
CC: Przemek Kitszel <[email protected]>
CC: [email protected]
CC: [email protected]
CC: Manish Chopra <[email protected]>
CC: [email protected]
CC: Kuniyuki Iwashima <[email protected]>
CC: Andrew Lunn <[email protected]>
CC: Nikolay Aleksandrov <[email protected]>
CC: [email protected]
Quoted 11 lines
drivers/net/ethernet/intel/ice/ice_main.c | 4 +++- drivers/net/ethernet/mscc/ocelot_net.c | 2 +- drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c | 2 +- drivers/net/macvlan.c | 2 +- drivers/net/vxlan/vxlan_core.c | 5 ++++- include/linux/netdevice.h | 9 +++++++-- net/bridge/br_fdb.c | 15 ++++++++------- net/bridge/br_private.h | 2 +- net/core/rtnetlink.c | 11 ++++++++--- 9 files changed, 34 insertions(+), 18 deletions(-)
Petr Machata
maintainerPATCH v3 3/7 selftests: net: lib: Move logging from forwarding/lib.sh here
In reply to Petr MachataMany net selftests invent their own logging helpers. These really should be in a library sourced by these tests. Currently forwarding/lib.sh has a suite of perfectly fine logging helpers, but sourcing a forwarding/ library from a higher-level directory smells of layering violation. In this patch, move the logging helpers to net/lib.sh so that every net test can use them.
Together with the logging helpers, it's also necessary to move pause_on_fail(), and EXIT_STATUS and RET.
Existing lib.sh users might be using these same names for their functions or variables. However lib.sh is always sourced near the top of the file (checked), and whatever new definitions will simply override the ones provided by lib.sh.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: [email protected]
CC: Jiri Pirko <[email protected]>
tools/testing/selftests/net/forwarding/lib.sh | 113 ----------------- tools/testing/selftests/net/lib.sh | 115 ++++++++++++++++++ 2 files changed, 115 insertions(+), 113 deletions(-)
tools/testing/selftests/net/forwarding/lib.sh +0 -113
@@ -48,7 +48,6 @@ declare -A NETIFS=( : "${WAIT_TIME:=5}" # Whether to pause on, respectively, after a failure and before cleanup. -: "${PAUSE_ON_FAIL:=no}" : "${PAUSE_ON_CLEANUP:=no}" # Whether to create virtual interfaces, and what netdevice type they should be.
@@ -446,22 +445,6 @@ done ############################################################################## # Helpers -# Exit status to return at the end. Set in case one of the tests fails. -EXIT_STATUS=0 -# Per-test return value. Clear at the beginning of each test. -RET=0 - -ret_set_ksft_status() -{ - local ksft_status=$1; shift - local msg=$1; shift - - RET=$(ksft_status_merge $RET $ksft_status) - if (( $? )); then - retmsg=$msg - fi -} - # Whether FAILs should be interpreted as XFAILs. Internal. FAIL_TO_XFAIL=
@@ -535,102 +518,6 @@ xfail_on_veth() fi } -log_test_result() -{ - local test_name=$1; shift - local opt_str=$1; shift - local result=$1; shift - local retmsg=$1; shift - - printf "TEST: %-60s [%s]\n" "$test_name $opt_str" "$result" - if [[ $retmsg ]]; then - printf "\t%s\n" "$retmsg" - fi -} - -pause_on_fail() -{ - if [[ $PAUSE_ON_FAIL == yes ]]; then - echo "Hit enter to continue, 'q' to quit" - read a - [[ $a == q ]] && exit 1 - fi -} - -handle_test_result_pass() -{ - local test_name=$1; shift - local opt_str=$1; shift - - log_test_result "$test_name" "$opt_str" " OK " -} - -handle_test_result_fail() -{ - local test_name=$1; shift - local opt_str=$1; shift - - log_test_result "$test_name" "$opt_str" FAIL "$retmsg" - pause_on_fail -} - -handle_test_result_xfail() -{ - local test_name=$1; shift - local opt_str=$1; shift - - log_test_result "$test_name" "$opt_str" XFAIL "$retmsg" - pause_on_fail -} - -handle_test_result_skip() -{ - local test_name=$1; shift - local opt_str=$1; shift - - log_test_result "$test_name" "$opt_str" SKIP "$retmsg" -} - -log_test() -{ - local test_name=$1 - local opt_str=$2 - - if [[ $# -eq 2 ]]; then - opt_str="($opt_str)" - fi - - if ((RET == ksft_pass)); then - handle_test_result_pass "$test_name" "$opt_str" - elif ((RET == ksft_xfail)); then - handle_test_result_xfail "$test_name" "$opt_str" - elif ((RET == ksft_skip)); then - handle_test_result_skip "$test_name" "$opt_str" - else - handle_test_result_fail "$test_name" "$opt_str" - fi - - EXIT_STATUS=$(ksft_exit_status_merge $EXIT_STATUS $RET) - return $RET -} - -log_test_skip() -{ - RET=$ksft_skip retmsg= log_test "$@" -} - -log_test_xfail() -{ - RET=$ksft_xfail retmsg= log_test "$@" -} - -log_info() -{ - local msg=$1 - - echo "INFO: $msg" -} - not() { "$@"
tools/testing/selftests/net/lib.sh +115 -0
130 lines not on this page · raw message
Signature · 1 line
2.45.0
Petr Machata
maintainerPATCH v3 4/7 selftests: net: lib: Move tests_run from forwarding/lib.sh here
In reply to Petr MachataIt would be good to use the same mechanism for scheduling and dispatching general net tests as the many forwarding tests already use. To that end, move the logging helpers to net/lib.sh so that every net test can use them.
Existing lib.sh users might be using the name themselves. However lib.sh is always sourced near the top of the file (checked), and whatever new definition will simply override the one provided by lib.sh.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: [email protected]
CC: Jiri Pirko <[email protected]>
tools/testing/selftests/net/forwarding/lib.sh | 10 ---------- tools/testing/selftests/net/lib.sh | 10 ++++++++++ 2 files changed, 10 insertions(+), 10 deletions(-)
tools/testing/selftests/net/forwarding/lib.sh +0 -10
@@ -1285,16 +1285,6 @@ matchall_sink_create() action drop } -tests_run() -{ - local current_test - - for current_test in ${TESTS:-$ALL_TESTS}; do - in_defer_scope \ - $current_test - done -} - cleanup() { pre_cleanup
tools/testing/selftests/net/lib.sh +10 -0
@@ -351,3 +351,13 @@ log_info() echo "INFO: $msg" } + +tests_run() +{ + local current_test + + for current_test in ${TESTS:-$ALL_TESTS}; do + in_defer_scope \ + $current_test + done +}
Signature · 1 line
2.45.0
Petr Machata
maintainerPATCH v3 5/7 selftests: net: lib: Move checks from forwarding/lib.sh here
In reply to Petr MachataFor logging to be useful, something has to set RET and retmsg by calling ret_set_ksft_status(). There is a suite of functions to that end in forwarding/lib: check_err, check_fail et.al. Move them to net/lib.sh so that every net test can use them.
Existing lib.sh users might be using these same names for their functions. However lib.sh is always sourced near the top of the file (checked), and whatever new definitions will simply override the ones provided by lib.sh.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: [email protected]
CC: Jiri Pirko <[email protected]>
tools/testing/selftests/net/forwarding/lib.sh | 73 ------------------- tools/testing/selftests/net/lib.sh | 73 +++++++++++++++++++ 2 files changed, 73 insertions(+), 73 deletions(-)
tools/testing/selftests/net/forwarding/lib.sh +0 -73
@@ -445,79 +445,6 @@ done ############################################################################## # Helpers -# Whether FAILs should be interpreted as XFAILs. Internal. -FAIL_TO_XFAIL= - -check_err() -{ - local err=$1 - local msg=$2 - - if ((err)); then - if [[ $FAIL_TO_XFAIL = yes ]]; then - ret_set_ksft_status $ksft_xfail "$msg" - else - ret_set_ksft_status $ksft_fail "$msg" - fi - fi -} - -check_fail() -{ - local err=$1 - local msg=$2 - - check_err $((!err)) "$msg" -} - -check_err_fail() -{ - local should_fail=$1; shift - local err=$1; shift - local what=$1; shift - - if ((should_fail)); then - check_fail $err "$what succeeded, but should have failed" - else - check_err $err "$what failed" - fi -} - -xfail() -{ - FAIL_TO_XFAIL=yes "$@" -} - -xfail_on_slow() -{ - if [[ $KSFT_MACHINE_SLOW = yes ]]; then - FAIL_TO_XFAIL=yes "$@" - else - "$@" - fi -} - -omit_on_slow() -{ - if [[ $KSFT_MACHINE_SLOW != yes ]]; then - "$@" - fi -} - -xfail_on_veth() -{ - local dev=$1; shift - local kind - - kind=$(ip -j -d link show dev $dev | - jq -r '.[].linkinfo.info_kind') - if [[ $kind = veth ]]; then - FAIL_TO_XFAIL=yes "$@" - else - "$@" - fi -} - not() { "$@"
tools/testing/selftests/net/lib.sh +73 -0
76 lines not on this page · raw message
Signature · 1 line
2.45.0
Petr Machata
maintainerPATCH v3 6/7 selftests: net: lib: Add kill_process
In reply to Petr MachataA number of selftests run processes in the background and need to kill them afterwards. Instead for everyone to open-code the kill / wait / redirect mantra, add a helper in net/lib.sh. Convert existing open-code sites.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: [email protected]
CC: Jiri Pirko <[email protected]>
CC: Andrew Lunn <[email protected]>
.../selftests/drivers/net/mlxsw/devlink_trap.sh | 2 +- .../drivers/net/mlxsw/devlink_trap_l3_drops.sh | 4 ++-- .../drivers/net/mlxsw/devlink_trap_l3_exceptions.sh | 12 ++++++------ .../drivers/net/mlxsw/devlink_trap_tunnel_ipip.sh | 4 ++-- .../drivers/net/mlxsw/devlink_trap_tunnel_ipip6.sh | 4 ++-- .../drivers/net/mlxsw/devlink_trap_tunnel_vxlan.sh | 4 ++-- .../net/mlxsw/devlink_trap_tunnel_vxlan_ipv6.sh | 4 ++-- .../testing/selftests/drivers/net/mlxsw/tc_sample.sh | 4 ++-- .../drivers/net/netdevsim/fib_notifications.sh | 6 +++--- tools/testing/selftests/net/drop_monitor_tests.sh | 2 +- tools/testing/selftests/net/fib_tests.sh | 8 ++++---- .../testing/selftests/net/forwarding/devlink_lib.sh | 2 +- tools/testing/selftests/net/forwarding/lib.sh | 3 +-- tools/testing/selftests/net/forwarding/tc_police.sh | 8 ++++---- tools/testing/selftests/net/lib.sh | 8 ++++++++ 15 files changed, 41 insertions(+), 34 deletions(-)
tools/testing/selftests/drivers/net/mlxsw/devlink_trap.sh +1 -1
@@ -116,7 +116,7 @@ dev_del_test() log_test "Device delete" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid } trap cleanup EXIT
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l3_drops.sh +2 -2
@@ -595,7 +595,7 @@ irif_disabled_test() log_test "Ingress RIF disabled" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid ip link set dev $rp1 nomaster __addr_add_del $rp1 add 192.0.2.2/24 2001:db8:1::2/64 ip link del dev br0 type bridge
@@ -645,7 +645,7 @@ erif_disabled_test() log_test "Egress RIF disabled" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid __addr_add_del $rp1 add 192.0.2.2/24 2001:db8:1::2/64 ip link del dev br0 type bridge devlink_trap_action_set $trap_name "drop"
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_l3_exceptions.sh +6 -6
@@ -202,7 +202,7 @@ mtu_value_is_too_small_test() mtu_restore $rp2 - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $h1 ingress protocol ip pref 1 handle 101 flower }
@@ -235,7 +235,7 @@ __ttl_value_is_too_small_test() log_test "TTL value is too small: TTL=$ttl_val" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $h1 ingress protocol ip pref 1 handle 101 flower }
@@ -299,7 +299,7 @@ __mc_reverse_path_forwarding_test() log_test "Multicast reverse path forwarding: $desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $rp2 egress protocol $proto pref 1 handle 101 flower }
@@ -347,7 +347,7 @@ __reject_route_test() log_test "Reject route: $desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid ip route del unreachable $unreachable tc filter del dev $h1 ingress protocol $proto pref 1 handle 101 flower }
@@ -542,7 +542,7 @@ ipv4_lpm_miss_test() log_test "LPM miss: IPv4" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid vrf_without_routes_destroy }
@@ -569,7 +569,7 @@ ipv6_lpm_miss_test() log_test "LPM miss: IPv6" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid vrf_without_routes_destroy }
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_ipip.sh +2 -2
@@ -176,7 +176,7 @@ ecn_decap_test() log_test "$desc: Inner ECN is not ECT and outer is $ecn_desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ip pref 1 handle 101 flower }
@@ -207,7 +207,7 @@ no_matching_tunnel_test() log_test "$desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ip pref 1 handle 101 flower }
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_ipip6.sh +2 -2
@@ -176,7 +176,7 @@ ecn_decap_test() log_test "$desc: Inner ECN is not ECT and outer is $ecn_desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ipv6 pref 1 handle 101 flower }
@@ -207,7 +207,7 @@ no_matching_tunnel_test() log_test "$desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ipv6 pref 1 handle 101 flower }
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_vxlan.sh +2 -2
@@ -183,7 +183,7 @@ ecn_decap_test() log_test "$desc: Inner ECN is not ECT and outer is $ecn_desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ip pref 1 handle 101 flower }
@@ -253,7 +253,7 @@ corrupted_packet_test() log_test "$desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ip pref 1 handle 101 flower }
tools/testing/selftests/drivers/net/mlxsw/devlink_trap_tunnel_vxlan_ipv6.sh +2 -2
@@ -188,7 +188,7 @@ ecn_decap_test() log_test "$desc: Inner ECN is not ECT and outer is $ecn_desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ipv6 pref 1 handle 101 flower }
@@ -262,7 +262,7 @@ corrupted_packet_test() log_test "$desc" - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $swp1 egress protocol ipv6 pref 1 handle 101 flower }
tools/testing/selftests/drivers/net/mlxsw/tc_sample.sh +2 -2
@@ -218,7 +218,7 @@ psample_capture_start() psample_capture_stop() { - { kill %% && wait %%; } 2>/dev/null + kill_process %% } __tc_sample_rate_test()
@@ -499,7 +499,7 @@ tc_sample_md_out_tc_occ_test() backlog=$(tc -j -p -s qdisc show dev $rp2 | jq '.[0]["backlog"]') # Kill mausezahn. - { kill %% && wait %%; } 2>/dev/null + kill_process %% psample_capture_stop
tools/testing/selftests/drivers/net/netdevsim/fib_notifications.sh +3 -3
@@ -94,7 +94,7 @@ route_addition_check() sleep 1 $IP route add $route dev dummy1 sleep 1 - kill %% && wait %% &> /dev/null + kill_process %% route_notify_check $outfile $expected_num_notifications $offload_failed rm -f $outfile
@@ -148,7 +148,7 @@ route_deletion_check() sleep 1 $IP route del $route dev dummy1 sleep 1 - kill %% && wait %% &> /dev/null + kill_process %% route_notify_check $outfile $expected_num_notifications rm -f $outfile
@@ -191,7 +191,7 @@ route_replacement_check() sleep 1 $IP route replace $route dev dummy2 sleep 1 - kill %% && wait %% &> /dev/null + kill_process %% route_notify_check $outfile $expected_num_notifications rm -f $outfile
tools/testing/selftests/net/drop_monitor_tests.sh +1 -1
@@ -77,7 +77,7 @@ sw_drops_test() rm ${dir}/packets.pcap - { kill %% && wait %%; } 2>/dev/null + kill_process %% timeout 5 dwdump -o sw -w ${dir}/packets.pcap (( $(tshark -r ${dir}/packets.pcap \ -Y 'ip.dst == 192.0.2.10' 2> /dev/null | wc -l) == 0))
tools/testing/selftests/net/fib_tests.sh +4 -4
@@ -689,7 +689,7 @@ fib6_notify_test() log_test $ret 0 "ipv6 route add notify" - { kill %% && wait %%; } 2>/dev/null + kill_process %% #rm errors.txt
@@ -736,7 +736,7 @@ fib_notify_test() log_test $ret 0 "ipv4 route add notify" - { kill %% && wait %%; } 2>/dev/null + kill_process %% rm errors.txt
@@ -2328,7 +2328,7 @@ ipv4_mangle_test() $IP route del table 123 172.16.101.0/24 dev veth1 $IP rule del pref 100 - { kill %% && wait %%; } 2>/dev/null + kill_process %% rm $tmp_file route_cleanup
@@ -2386,7 +2386,7 @@ ipv6_mangle_test() $IP -6 route del table 123 2001:db8:101::/64 dev veth1 $IP -6 rule del pref 100 - { kill %% && wait %%; } 2>/dev/null + kill_process %% rm $tmp_file route_cleanup
tools/testing/selftests/net/forwarding/devlink_lib.sh +1 -1
@@ -501,7 +501,7 @@ devlink_trap_drop_cleanup() local pref=$1; shift local handle=$1; shift - kill $mz_pid && wait $mz_pid &> /dev/null + kill_process $mz_pid tc filter del dev $dev egress protocol $proto pref $pref handle $handle flower }
tools/testing/selftests/net/forwarding/lib.sh +1 -2
@@ -1574,8 +1574,7 @@ stop_traffic() { local pid=${1-%%}; shift - # Suppress noise from killing mausezahn. - { kill $pid && wait $pid; } 2>/dev/null + kill_process "$pid" } declare -A cappid
tools/testing/selftests/net/forwarding/tc_police.sh +4 -4
@@ -148,7 +148,7 @@ police_common_test() log_test "$test_name" - { kill %% && wait %%; } 2>/dev/null + kill_process %% tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower }
@@ -198,7 +198,7 @@ police_shared_common_test() log_test "$test_name" - { kill %% && wait %%; } 2>/dev/null + kill_process %% } police_shared_test()
@@ -278,7 +278,7 @@ police_mirror_common_test() log_test "$test_name" - { kill %% && wait %%; } 2>/dev/null + kill_process %% tc filter del dev $pol_if $dir protocol ip pref 1 handle 101 flower tc filter del dev $h3 ingress protocol ip pref 1 handle 101 flower tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower
@@ -320,7 +320,7 @@ police_pps_common_test() log_test "$test_name" - { kill %% && wait %%; } 2>/dev/null + kill_process %% tc filter del dev $h2 ingress protocol ip pref 1 handle 101 flower }
tools/testing/selftests/net/lib.sh +8 -0
@@ -434,3 +434,11 @@ xfail_on_veth() "$@" fi } + +kill_process() +{ + local pid=$1; shift + + # Suppress noise from killing the process. + { kill $pid && wait $pid; } 2>/dev/null +}
Signature · 1 line
2.45.0
Petr Machata
maintainerPATCH v3 7/7 selftests: net: fdb_notify: Add a test for FDB notifications
In reply to Petr MachataCheck that only one notification is produced for various FDB edit operations.
Regarding the ip_link_add() and ip_link_master() helpers. This pattern of action plus corresponding defer is bound to come up often, and a dedicated vocabulary to capture it will be handy. tunnel_create() and vlan_create() from forwarding/lib.sh are somewhat opaque and perhaps too kitchen-sinky, so I tried to go in the opposite direction with these ones, and wrapped only the bare minimum to schedule a corresponding cleanup.
---
Notes:
CC: Shuah Khan <[email protected]>
CC: Benjamin Poirier <[email protected]>
CC: Hangbin Liu <[email protected]>
CC: [email protected]
CC: Jiri Pirko <[email protected]>
tools/testing/selftests/net/Makefile | 2 +- tools/testing/selftests/net/fdb_notify.sh | 95 +++++++++++++++++++++++ tools/testing/selftests/net/lib.sh | 17 ++++ 3 files changed, 113 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/net/fdb_notify.sh
tools/testing/selftests/net/Makefile +1 -1
@@ -92,7 +92,7 @@ TEST_PROGS += test_vxlan_mdb.sh TEST_PROGS += test_bridge_neigh_suppress.sh TEST_PROGS += test_vxlan_nolocalbypass.sh TEST_PROGS += test_bridge_backup_port.sh -TEST_PROGS += fdb_flush.sh +TEST_PROGS += fdb_flush.sh fdb_notify.sh TEST_PROGS += fq_band_pktlimit.sh TEST_PROGS += vlan_hw_filter.sh TEST_PROGS += bpf_offload.py
tools/testing/selftests/net/fdb_notify.sh new file +95 -0
95 lines not on this page · raw message
tools/testing/selftests/net/lib.sh +17 -0
@@ -442,3 +442,20 @@ kill_process() # Suppress noise from killing the process. { kill $pid && wait $pid; } 2>/dev/null } + +ip_link_add() +{ + local name=$1; shift + + ip link add name "$name" "$@" + defer ip link del dev "$name" +} + +ip_link_master() +{ + local member=$1; shift + local master=$1; shift + + ip link set dev "$member" master "$master" + defer ip link set dev "$member" nomaster +}
Signature · 1 line
2.45.0
Jakub Kicinski
maintainerRe: [PATCH net-next v3 7/7] selftests: net: fdb_notify: Add a test for FDB notifications
Quoted 9 lines · On Mon, 11 Nov 2024 18:09:01 +0100 Petr Machata wrote:
Check that only one notification is produced for various FDB edit operations.
Regarding the
ip_link_add()andip_link_master()helpers. This pattern of action plus corresponding defer is bound to come up often, and a dedicated vocabulary to capture it will be handy.tunnel_create()andvlan_create()fromforwarding/lib.share somewhat opaque and perhaps too kitchen-sinky, so I tried to go in the opposite direction with these ones, and wrapped only the bare minimum to schedule a corresponding cleanup.
Looks like it fails about half of the time :(
https://netdev.bots.linux.dev/flakes.html?min-flip=0&tn-needle=fdb-notify&br-cnt=200
Signature · 1 line
pw-bot: cr
Petr Machata
maintainerRe: [PATCH net-next v3 7/7] selftests: net: fdb_notify: Add a test for FDB notifications
Quoted 11 lines · Jakub Kicinski
Quoted 9 lines · On Mon, 11 Nov 2024 18:09:01 +0100 Petr Machata wrote:
Check that only one notification is produced for various FDB edit operations.
Regarding the
ip_link_add()andip_link_master()helpers. This pattern of action plus corresponding defer is bound to come up often, and a dedicated vocabulary to capture it will be handy.tunnel_create()andvlan_create()fromforwarding/lib.share somewhat opaque and perhaps too kitchen-sinky, so I tried to go in the opposite direction with these ones, and wrapped only the bare minimum to schedule a corresponding cleanup.
Quoted 3 lines
Looks like it fails about half of the time :(
https://netdev.bots.linux.dev/flakes.html?min-flip=0&tn-needle=fdb-notify&br-cnt=200
OK, I can't reproduce this. Trying in VM, on an actual HW, debug, no debug, no luck. But I see basically two failures:
- A "0 seen, 1 expected", which... I don't know, maybe it could just be a misplaced sleep. I don't see how, but it's a deterministing scenario, there shouldn't be anything racy here, either it emits or it doesn't, so some buffering issue is the only thing I can think of.
- Deadlocks. E.g. this, which looks like it deadlocked and timed out ("bad unlock balance detected" followed by "blocked for more than 122 seconds" et.al.):
https://netdev-3.bots.linux.dev/vmksft-net-dbg/results/846621/18-fdb-notify-sh/
Like... how could this patchset even theoretically cause a deadlock?
Petr Machata
maintainerRe: [PATCH net-next v3 7/7] selftests: net: fdb_notify: Add a test for FDB notifications
Quoted 17 lines · Petr Machata
Quoted 11 lines · Jakub Kicinski
Quoted 9 lines · On Mon, 11 Nov 2024 18:09:01 +0100 Petr Machata wrote:
Check that only one notification is produced for various FDB edit operations.
Regarding the
ip_link_add()andip_link_master()helpers. This pattern of action plus corresponding defer is bound to come up often, and a dedicated vocabulary to capture it will be handy.tunnel_create()andvlan_create()fromforwarding/lib.share somewhat opaque and perhaps too kitchen-sinky, so I tried to go in the opposite direction with these ones, and wrapped only the bare minimum to schedule a corresponding cleanup.Quoted 3 lines
Looks like it fails about half of the time :(
https://netdev.bots.linux.dev/flakes.html?min-flip=0&tn-needle=fdb-notify&br-cnt=200
Quoted 7 lines
OK, I can't reproduce this. Trying in VM, on an actual HW, debug, no debug, no luck. But I see basically two failures:
- A "0 seen, 1 expected", which... I don't know, maybe it could just be a misplaced sleep. I don't see how, but it's a deterministing scenario, there shouldn't be anything racy here, either it emits or it doesn't, so some buffering issue is the only thing I can think of.
I think this really could be just a "bridge monitor" taking a bit more time to start every now and then. Can I have you test with this extra chunk, or should I just resend with that change and hope for the best?
tools/testing/selftests/net/fdb_notify.sh +1 -0
@@ -26,6 +26,7 @@ do_test_dup() bridge monitor fdb &> "$tmpf" & defer kill_process $! + sleep 0.5 bridge fdb "$op" 00:11:22:33:44:55 vlan 1 "$@" sleep 0.2 defer_scope_pop
Quoted 1 line
- Deadlocks. E.g. this, which looks like it deadlocked and timed out
Eh, these are ancient. Never mind.
Jakub Kicinski
maintainerRe: [PATCH net-next v3 7/7] selftests: net: fdb_notify: Add a test for FDB notifications
Quoted 8 lines · On Wed, 13 Nov 2024 16:11:03 +0100 Petr Machata wrote:
Quoted 4 lines
- A "0 seen, 1 expected", which... I don't know, maybe it could just be a misplaced sleep. I don't see how, but it's a deterministing scenario, there shouldn't be anything racy here, either it emits or it doesn't, so some buffering issue is the only thing I can think of.
I think this really could be just a "bridge monitor" taking a bit more time to start every now and then. Can I have you test with this extra chunk, or should I just resend with that change and hope for the best?
Let's give it a go, if it doesn't fix it we can try to do sneaky local changes in the CI, without more resends.