On 16/10/2024 10:59, Daniel Borkmann wrote:
> On 10/16/24 5:16 AM, Hangbin Liu wrote:
>> Bonding only supports native XDP for specific modes, which can lead to
>> confusion for users regarding why XDP loads successfully at times and
>> fails at others. This patch enhances error handling by returning detailed
>> error messages, providing users with clearer insights into the specific
>> reasons for the failure when loading native XDP.
>>
>> Signed-off-by: Hangbin Liu <liuhangbin@xxxxxxxxx>
>> ---
>> drivers/net/bonding/bond_main.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index b1bffd8e9a95..f0f76b6ac8be 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -5676,8 +5676,11 @@ static int bond_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>> ASSERT_RTNL();
>> - if (!bond_xdp_check(bond))
>> + if (!bond_xdp_check(bond)) {
>> + BOND_NL_ERR(dev, extack,
>> + "No native XDP support for the current bonding mode");
>> return -EOPNOTSUPP;
>> + }
>> old_prog = bond->xdp_prog;
>> bond->xdp_prog = prog;
>
> LGTM, but independent of these I was more thinking whether something like this
> could do the trick (only compile tested). That way you also get the fallback
> without changing anything in the core XDP code.
>
> Thanks,
> Daniel
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index b1bffd8e9a95..2861b3a895ff 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -5915,6 +5915,10 @@ static const struct ethtool_ops bond_ethtool_ops = {
> .get_ts_info = bond_ethtool_get_ts_info,
> };
>
> +static const struct device_type bond_type = {
> + .name = "bond",
> +};
> +
> static const struct net_device_ops bond_netdev_ops = {
> .ndo_init = bond_init,
> .ndo_uninit = bond_uninit,
> @@ -5951,9 +5955,20 @@ static const struct net_device_ops bond_netdev_ops = {
> .ndo_hwtstamp_set = bond_hwtstamp_set,
> };
>
> -static const struct device_type bond_type = {
> - .name = "bond",
> -};
> +static struct net_device_ops bond_netdev_ops_noxdp __ro_after_init;
> +
> +static void __init bond_setup_noxdp_ops(void)
> +{
> + memcpy(&bond_netdev_ops_noxdp, &bond_netdev_ops,
> + sizeof(bond_netdev_ops));
> +
> + /* Used for bond device mode which does not support XDP
> + * yet, see also bond_xdp_check().
> + */
> + bond_netdev_ops_noxdp.ndo_bpf = NULL;
> + bond_netdev_ops_noxdp.ndo_xdp_xmit = NULL;
> + bond_netdev_ops_noxdp.ndo_xdp_get_xmit_slave = NULL;
> +}
>
> static void bond_destructor(struct net_device *bond_dev)
> {
> @@ -5978,7 +5993,9 @@ void bond_setup(struct net_device *bond_dev)
> /* Initialize the device entry points */
> ether_setup(bond_dev);
> bond_dev->max_mtu = ETH_MAX_MTU;
> - bond_dev->netdev_ops = &bond_netdev_ops;
> + bond_dev->netdev_ops = bond_xdp_check(bond) ?
> + &bond_netdev_ops :
> + &bond_netdev_ops_noxdp;
This will have to be done safely on bond mode change as well.
If all slaves are released we can switch modes without destroying
the device.
> bond_dev->ethtool_ops = &bond_ethtool_ops;
>
> bond_dev->needs_free_netdev = true;
> @@ -6591,6 +6608,8 @@ static int __init bonding_init(void)
> int i;
> int res;
>
> + bond_setup_noxdp_ops();
> +
> res = bond_check_params(&bonding_defaults);
> if (res)
> goto out;
