Re: [PATCH v2 net] bonding: add xdp_features support

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5700 bytes --]

> On 4/30/23 12:02 PM, Lorenzo Bianconi wrote:
> > Introduce xdp_features support for bonding driver according to the slave
> > devices attached to the master one. xdp_features is required whenever we
> > want to xdp_redirect traffic into a bond device and then into selected
> > slaves attached to it.
> > 
> > Fixes: 66c0e13ad236 ("drivers: net: turn on XDP features")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Please also keep Jussi in Cc for bonding + XDP reviews [added here].

ack, will do

> 
> > ---
> > Change since v1:
> > - remove bpf self-test patch from the series
> 
> Given you targeted net tree, was this patch run against BPF CI locally from
> your side to avoid breakage again?

yes, I tested it locally and opening a PR upstream [0] (upstream xdp bonding
tests are fine but the PR fails, however the issue seems not related to the
code I added and the error is in common even with some previous pending PR).

Regards,
Lorenzo

[0] https://github.com/kernel-patches/bpf/pull/5021

> 
> Thanks,
> Daniel
> 
> > ---
> >   drivers/net/bonding/bond_main.c    | 48 ++++++++++++++++++++++++++++++
> >   drivers/net/bonding/bond_options.c |  2 ++
> >   include/net/bonding.h              |  1 +
> >   3 files changed, 51 insertions(+)
> > 
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 710548dbd0c1..c98121b426a4 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -1789,6 +1789,45 @@ static void bond_ether_setup(struct net_device *bond_dev)
> >   	bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
> >   }
> > +void bond_xdp_set_features(struct net_device *bond_dev)
> > +{
> > +	struct bonding *bond = netdev_priv(bond_dev);
> > +	xdp_features_t val = NETDEV_XDP_ACT_MASK;
> > +	struct list_head *iter;
> > +	struct slave *slave;
> > +
> > +	ASSERT_RTNL();
> > +
> > +	if (!bond_xdp_check(bond)) {
> > +		xdp_clear_features_flag(bond_dev);
> > +		return;
> > +	}
> > +
> > +	bond_for_each_slave(bond, slave, iter) {
> > +		struct net_device *dev = slave->dev;
> > +
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_BASIC)) {
> > +			xdp_clear_features_flag(bond_dev);
> > +			return;
> > +		}
> > +
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_REDIRECT))
> > +			val &= ~NETDEV_XDP_ACT_REDIRECT;
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT))
> > +			val &= ~NETDEV_XDP_ACT_NDO_XMIT;
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_XSK_ZEROCOPY))
> > +			val &= ~NETDEV_XDP_ACT_XSK_ZEROCOPY;
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_HW_OFFLOAD))
> > +			val &= ~NETDEV_XDP_ACT_HW_OFFLOAD;
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_RX_SG))
> > +			val &= ~NETDEV_XDP_ACT_RX_SG;
> > +		if (!(dev->xdp_features & NETDEV_XDP_ACT_NDO_XMIT_SG))
> > +			val &= ~NETDEV_XDP_ACT_NDO_XMIT_SG;
> > +	}
> > +
> > +	xdp_set_features_flag(bond_dev, val);
> > +}
> > +
> >   /* enslave device <slave> to bond device <master> */
> >   int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> >   		 struct netlink_ext_ack *extack)
> > @@ -2236,6 +2275,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> >   			bpf_prog_inc(bond->xdp_prog);
> >   	}
> > +	bond_xdp_set_features(bond_dev);
> > +
> >   	slave_info(bond_dev, slave_dev, "Enslaving as %s interface with %s link\n",
> >   		   bond_is_active_slave(new_slave) ? "an active" : "a backup",
> >   		   new_slave->link != BOND_LINK_DOWN ? "an up" : "a down");
> > @@ -2483,6 +2524,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> >   	if (!netif_is_bond_master(slave_dev))
> >   		slave_dev->priv_flags &= ~IFF_BONDING;
> > +	bond_xdp_set_features(bond_dev);
> >   	kobject_put(&slave->kobj);
> >   	return 0;
> > @@ -3930,6 +3972,9 @@ static int bond_slave_netdev_event(unsigned long event,
> >   		/* Propagate to master device */
> >   		call_netdevice_notifiers(event, slave->bond->dev);
> >   		break;
> > +	case NETDEV_XDP_FEAT_CHANGE:
> > +		bond_xdp_set_features(bond_dev);
> > +		break;
> >   	default:
> >   		break;
> >   	}
> > @@ -5874,6 +5919,9 @@ void bond_setup(struct net_device *bond_dev)
> >   	if (BOND_MODE(bond) == BOND_MODE_ACTIVEBACKUP)
> >   		bond_dev->features |= BOND_XFRM_FEATURES;
> >   #endif /* CONFIG_XFRM_OFFLOAD */
> > +
> > +	if (bond_xdp_check(bond))
> > +		bond_dev->xdp_features = NETDEV_XDP_ACT_MASK;
> >   }
> >   /* Destroy a bonding device.
> > diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> > index f71d5517f829..0498fc6731f8 100644
> > --- a/drivers/net/bonding/bond_options.c
> > +++ b/drivers/net/bonding/bond_options.c
> > @@ -877,6 +877,8 @@ static int bond_option_mode_set(struct bonding *bond,
> >   			netdev_update_features(bond->dev);
> >   	}
> > +	bond_xdp_set_features(bond->dev);
> > +
> >   	return 0;
> >   }
> > diff --git a/include/net/bonding.h b/include/net/bonding.h
> > index c3843239517d..a60a24923b55 100644
> > --- a/include/net/bonding.h
> > +++ b/include/net/bonding.h
> > @@ -659,6 +659,7 @@ void bond_destroy_sysfs(struct bond_net *net);
> >   void bond_prepare_sysfs_group(struct bonding *bond);
> >   int bond_sysfs_slave_add(struct slave *slave);
> >   void bond_sysfs_slave_del(struct slave *slave);
> > +void bond_xdp_set_features(struct net_device *bond_dev);
> >   int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
> >   		 struct netlink_ext_ack *extack);
> >   int bond_release(struct net_device *bond_dev, struct net_device *slave_dev);
> > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]