netdev call - Mar 5th

Hi,

The bi-weekly netdev call at https://bbb.lwn.net/b/jak-wkr-seg-hjn
is scheduled tomorrow at 8:30 am (PT) / 5:30 pm (~EU).

No topics so far, please reach out if you have something to discuss, otherwise we'll cancel.

The reviewer of the week is.. Meta!

Sorry, ignore that, ENOCOFFEE.
The next call is scheduled for the 12th of March, not tomorrow 🤦️

We have a topic for the 12.03.2024 netdev call:

How set_channels() input should be interpreted

Mostly, implemetations of .set_channels() are tailored for only one of the following inputs:
1. ethtool -L <ifname> combined <n>
2. ethtool -L <ifname> rx <n> tx <m>

There are also exceptions, e.g.
* bnxt: input 1 puts interface in a shared channels mode, input 2 puts device into a dedicated channels mode
* xgbe: ethtool -L <ifname> combined <n> rx <m> tx <l>, <m> and <l> cannot both be non-zero at the same time
* ice and idpf: accept both inputs, but try to create as many queue pairs as possible

In idpf and ice, we have encountered a problem, where the checks (listed below) in ethnl_set_channels() do not trigger for us, because our set_channels() operates under an assumption that if a value in ethtool_channels stays the same, it was not provided by user and therefore defaults to '0' [0].

The ethnl_set_channels() checks and how they fail us:
1. Ensure there is at least 1 RX and 1 TX channel [1]
ethtool -l <ifname> -> combined 40
ethtool -L <ifname> combined 40
ice triggers an internal check, which was removed from idpf after review [2], so idpf tries to set 0 RX and
0 TX queues
2. Prevent disabling a channel with an AF_XDP socket attached [3]

        ethtool -l <ifname> -> combined 40
        ./xdpsock -i <ifname> -q35
        ethtool -L <ifname> rx 30 tx 30

ice destroys the queue the socket is attached to, despite there being a check in the core code that must prevent this [3]
In theory, the same should be possible to do the same on e.g. mlx5, by doing
ethtool -L <ifname> combined 30 rx 10 tx 10

So, we would like to discuss, how can we solve the above problem and also establish how should an ideal set_channels() input interpretation look.

Possible solutions:
1. Convert our drivers to whatever is a preferred way to handle the input.
2. Add .fix_channels() callback to ethtool_ops

        [...]
        mod |= mod_combined;
        if (!mod)
                return 0;

        /* Here ice and idpf would change unmodified values to 0 and convert
         * separate overlapping rx/tx channels into combined,
         * mlx5 and bnxt would set rx_count and tx_count to 0, etc.
         */
        if (dev->ethtool_ops->fix_num_channels)
                dev->ethtool_ops->fix_num_channels(dev, &channels);

        /* ethtool does generic checks on the modified values and calls
         * set_channels() after
         */
        [...]

3. Use the abovementioned assumption made by ice driver [0] in ethnl_set_channels(). We propose this, because no doubt about it was expressed during the community review of the both affected drivers [2][4].

[0] https://elixir.bootlin.com/linux/v6.8-rc7/source/drivers/net/ethernet/intel/ice/ice_ethtool.c#L3541
[1] https://elixir.bootlin.com/linux/v6.8-rc7/source/net/ethtool/channels.c#L154
[2] https://lore.kernel.org/netdev/[email protected]/
[3] https://elixir.bootlin.com/linux/v6.8-rc7/source/net/ethtool/channels.c#L186
[4] https://lore.kernel.org/netdev/[email protected]/

Great! Let me change the subject perhaps in an attempt to turn this into the reminder:

The bi-weekly netdev call at https://bbb.lwn.net/b/jak-wkr-seg-hjn
is scheduled tomorrow at 8:30 am (PT) / 4:30 pm (~EU).

NOTE NOTE NOTE: we are already on summer time in the US. So for for those who switch time at different points this call will likely be at an unusual hour for you!