Re: [PATCH net-next] net: libwx: Add ethtool -L support for VF drivers

["mengyuanlou@net-swift.com" <mengyuanlou@net-swift.com>]

> 2026年9月16日 03:19，Harshitha Ramamurthy <hramamurthy@google.com> 写道：
> 
> On Tue, Sep 15, 2026 at 3:18 AM Mengyuan Lou <mengyuanlou@net-swift.com> wrote:
>> 
>> Implement get_channels and set_channels ethtool operations for Wangxun
>> VF net_device instances in libwx.
>> 
>> This allows users to query and dynamically reconfigure the number of RX/TX
>> channels on VF interfaces via `ethtool -l/ -L`.
>> 
>> Specifically:
>> - Add wx_get_channels_vf() and wx_set_channels_vf() to report and update
>>  channel limits and combined queue counts.
>> - Re-evaluate RSS limits in txgbevf_set_num_queues() based on user settings
>>  stored in wx->ring_feature[RING_F_RSS].limit.
>> - Adjust VF MRQC RSS hash mask logic in wx_setup_vfmrqc_vf() to correctly
>>  match queue configuration thresholds.
>> 
>> Signed-off-by: Mengyuan Lou <mengyuanlou@net-swift.com>
>> ---
>> .../net/ethernet/wangxun/libwx/wx_ethtool.c   | 64 +++++++++++++++++++
>> .../net/ethernet/wangxun/libwx/wx_vf_lib.c    |  2 +-
>> .../ethernet/wangxun/txgbevf/txgbevf_main.c   |  3 +
>> 3 files changed, 68 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
>> index 940d2e59876c..d24370790941 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_ethtool.c
>> @@ -564,6 +564,68 @@ int wx_set_channels(struct net_device *dev,
>> }
>> EXPORT_SYMBOL(wx_set_channels);
>> 
>> +static void wx_get_channels_vf(struct net_device *dev,
>> +                              struct ethtool_channels *ch)
>> +{
>> +       struct wx *wx = netdev_priv(dev);
>> +
>> +       /* report maximum channels */
>> +       ch->max_combined = wx->mac.max_rx_queues;
>> +
>> +       ch->max_other = 1;
>> +       ch->other_count = 1;
>> +
>> +       /* record current channels */
>> +       ch->combined_count = wx->num_rx_queues;
>> +}
>> +
>> +static int wx_set_channels_vf(struct net_device *dev,
>> +                             struct ethtool_channels *ch)
>> +{
>> +       struct wx *wx = netdev_priv(dev);
>> +       u16 rss_limit;
>> +       int ret;
>> +
>> +       rss_limit = wx->ring_feature[RING_F_RSS].limit;
>> +
>> +       /* verify other_count has not changed */
>> +       if (ch->other_count != 1)
>> +               return -EINVAL;
>> +
>> +       /* verify the number of channels does not exceed hardware limits */
>> +       if (ch->combined_count > wx->mac.max_rx_queues)
>> +               return -EINVAL;
>> +
>> +       /* if device is resetting, do nothing */
>> +       if (test_bit(WX_STATE_RESETTING, wx->state))
>> +               return -EBUSY;
>> +
>> +       wx->ring_feature[RING_F_RSS].limit = ch->combined_count;
>> +
>> +       if (netif_running(dev))
>> +               wxvf_close(dev);
> 
> The precedent has been for some time now that drivers are required to
> pre-allocate resources for the new queue counts before tearing down
> the existing ones. So that the driver doesn't knock itself out if
> something fails when attempting to apply the new configuration.
> 
> Look at the nfp_net_ring_config() or gve_adjust_config() for examples:
> both try to allocate memory for the new config first.

Thanks for the feedback.

I understand that pre-allocating resources before tearing down existing queues
("prepare-then-commit") is the ideal pattern used by drivers like NFP and GVE
to ensure zero-downtime or graceful failure recovery during queue reconfigurations.

However, adopting the NFP/GVE pre-allocation model is not technically feasible
for WangXun VF without closing the device first.

1. MSI-X Vector Lifecycle Constraints
	Unlike NFP (which can dynamically manage dual sets of MSI-X vectors) or GVE
(which pre-allocates all possible MSI-X vectors at probe time and only resizes
queues afterwards), WangXun VF acquires its MSI-X vectors in a single lump sum
via pci_alloc_irq_vectors_affinity().
    • To change the vector count or reconfigure queue-to-vector mappings, Linux
	PCI architecture requires freeing all active vectors via pci_free_irq_vectors()
	before requesting new ones.
    • Freeing active MSI-X vectors requires teardown of the current interrupt scheme,
	which cannot be safely done while the interface remains RUNNING.
2. NAPI Registration Constraints (netif_napi_add)
	Each queue vector allocates a struct wx_q_vector and registers a NAPI instance via
netif_napi_add().
    • In the WangXun driver architecture, NAPI instances are tightly bound to the
	queue/vector layout.
    • Allocating new q_vectors and invoking netif_napi_add() while the driver is running
	risks duplicate NAPI registrations or corrupted polling state. Therefore, the
	device must be down (wxvf_close) before cleaning up old NAPI instances and
	instantiating new ones.



%%{init: {'theme':'base','themeVariables':{'primaryColor':'#dd6b20','edgeLabelBackground':'#fff','clusterBkg':'#fff5f5','clusterBorder':'#fdba74'}}}%%
graph TD
    %% --------------------------------------------------------------
    %% Entry & parameter checks
    %% --------------------------------------------------------------
    A[wx_set_channels_vf(dev, ch)]
    A --> B[if (ch->other_count != 1)  →  -EINVAL]
    B --> C[if (ch->combined_count > wx->mac.max_rx_queues)  →  -EINVAL]
    C --> D[if (test_bit(WX_STATE_RESETTING, wx->state))  →  -EBUSY]

    %% --------------------------------------------------------------
    %% Must stop the VF first if it is running
    %% --------------------------------------------------------------
    D -->|netif_running(dev)| E[wxvf_close(dev)   // stop Tx/Rx, disable NAPI, free resources]
    D -->|!netif_running(dev)| F[(skip) ]

    %% --------------------------------------------------------------
    %% Completely clear the current interrupt scheme (including MSI‑X)
    %% --------------------------------------------------------------
    E --> G[wx_clear_interrupt_scheme(wx)]
    subgraph G[wx_clear_interrupt_scheme(wx) details]
        direction TB
        G1[wx_free_q_vectors(wx)                 // free q_vectors only]
        G2[wx_reset_interrupt_capability(wx)    // free MSI‑X vectors (pci_free_irq_vectors)]
        G1 --> G2
    end

    %% --------------------------------------------------------------
    %% Re‑initialize the whole interrupt scheme (allocate MSI‑X, q_vectors, rings)
    %% --------------------------------------------------------------
    G --> H[wx_init_interrupt_scheme(wx)]
    subgraph H[wx_init_interrupt_scheme(wx) details]
        direction TB
        H1[wx_set_num_queues(wx)                 // recompute num_tx/num_rx/num_q_vectors]
        H2[wx_set_interrupt_capability(wx)       // request MSI‑X vectors (pci_alloc_irq_vectors)]
        H3[wx_alloc_q_vectors(wx)               // allocate q_vector structs and rings (netif_napi_add)]
        H4[wx_cache_ring_rss(wx)                 // compute RSS reg_idx and program hardware]
        H5[set_bit(WX_STATE_DOWN, wx->state)]
        H1 --> H2 --> H3 --> H4 --> H5
    end

    %% --------------------------------------------------------------
    %% If the device was running before, open it again
    %% --------------------------------------------------------------
    H -->|netif_running(dev) after re‑init| I[wxvf_open(dev)   // re‑enable NAPI, wake Tx/Rx]
    H -->|!netif_running(dev) after re‑init| J[return 0]

    I --> K[return 0  // success]
    J --> K

    %% --------------------------------------------------------------
    %% Error path – restore old RSS limit and return error
    %% --------------------------------------------------------------
    H -->|ret != 0| L[restore original RSS limit & return error]

    %% --------------------------------------------------------------
    %% Conflict points (why we cannot “prepare‑first, close‑later”)
    %% --------------------------------------------------------------
    classDef note fill:#fffae6,color:#333,stroke:#ffa726;


    N1[**Conflict 1**: MSI‑X vectors are allocated only once (wx_set_interrupt_capability) – a new allocation requires pci_free_irq_vectors(), which can be done only after the old vectors are released (i.e. after the device is closed).]:::note
    N2[**Conflict 2**: netif_napi_add() (called inside wx_alloc_q_vectors) must be executed while the device is not RUNNING; otherwise double‑registration of NAPI leads to bugs.]:::note
    N3[**Conflict 3**: Hardware register mapping (reg_idx, RSS tables) must be programmed before the device is opened; changing them while the hardware is running causes race‑conditions.]:::note

    N1 -.-> H2
    N2 -.-> H3
    N3 -.-> H4





 |
|------|----------|----------|
| **NFP** | `nfp_net_set_channels()` → `nfp_net_ring_reconfig()`

**GVE** | `gve_set_channels()` → `gve_adjust_queues()` → `gve_adjust_config()`

NFP
 * struct nfp_net_dp - NFP network device datapath data structure

gve
   struct gve_tx_alloc_rings_cfg tx_alloc_cfg = {0};
   struct gve_rx_alloc_rings_cfg rx_alloc_cfg = {0};

The allocation of interrupt resources and ring resources can be separated
which Wangxun vf can not do it.







> 
>> +
>> +       wx_clear_interrupt_scheme(wx);
>> +
>> +       ret = wx_init_interrupt_scheme(wx);
>> +       if (ret)
>> +               goto err_out;
>> +
>> +       if (netif_running(dev)) {
>> +               ret = wxvf_open(dev);
>> +               if (ret)
>> +                       goto err_close;
>> +       }
>> +
>> +       return 0;
>> +
>> +err_close:
>> +       wx_clear_interrupt_scheme(wx);
>> +err_out:
>> +       wx->ring_feature[RING_F_RSS].limit = rss_limit;
>> +       return ret;
>> +}
>> +
>> u32 wx_rss_indir_size(struct net_device *netdev)
>> {
>>        struct wx *wx = netdev_priv(netdev);
>> @@ -852,6 +914,8 @@ static const struct ethtool_ops wx_ethtool_ops_vf = {
>>        .set_coalesce           = wx_set_coalesce,
>>        .get_ts_info            = ethtool_op_get_ts_info,
>>        .get_link_ksettings     = wx_get_link_ksettings_vf,
>> +       .get_channels           = wx_get_channels_vf,
>> +       .set_channels           = wx_set_channels_vf,
>> };
>> 
>> void wx_set_ethtool_ops_vf(struct net_device *netdev)
>> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_vf_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_vf_lib.c
>> index 7325b475ee10..6fec3ab5e4e4 100644
>> --- a/drivers/net/ethernet/wangxun/libwx/wx_vf_lib.c
>> +++ b/drivers/net/ethernet/wangxun/libwx/wx_vf_lib.c
>> @@ -237,7 +237,7 @@ void wx_setup_vfmrqc_vf(struct wx *wx)
>> 
>>        vfmrqc |= WX_VXMRQC_RSS_EN;
>> 
>> -       if (wx->num_rx_queues > 3)
>> +       if (wx->num_rx_queues >= 3)
>>                vfmrqc |= WX_VXMRQC_RSS_HASH(2);
>>        else if (wx->num_rx_queues > 1)
>>                vfmrqc |= WX_VXMRQC_RSS_HASH(1);
>> diff --git a/drivers/net/ethernet/wangxun/txgbevf/txgbevf_main.c b/drivers/net/ethernet/wangxun/txgbevf/txgbevf_main.c
>> index 8b16b900820a..6e8659082051 100644
>> --- a/drivers/net/ethernet/wangxun/txgbevf/txgbevf_main.c
>> +++ b/drivers/net/ethernet/wangxun/txgbevf/txgbevf_main.c
>> @@ -72,6 +72,9 @@ static void txgbevf_set_num_queues(struct wx *wx)
>>                queue = min_t(u16, wx->mac.max_rx_queues, wx->mac.max_tx_queues);
>>                rss = min_t(u16, queue, rss);
>> 
>> +               if (wx->ring_feature[RING_F_RSS].limit)
>> +                       rss = min_t(u16, rss, wx->ring_feature[RING_F_RSS].limit);
>> +
>>                if (wx->vfinfo->vf_api >= wx_mbox_api_13) {
>>                        wx->num_rx_queues = rss;
>>                        wx->num_tx_queues = rss;
>> --
>> 2.30.1
>> 
>> 
>> 
> 
>