Hello,
On Fri, Sep 22, 2023 at 4:41 AM Jie Hai <haij...@huawei.com> wrote:
> On 2023/9/19 0:54, Ferruh Yigit wrote:
> > On 9/8/2023 12:50 PM, David Marchand wrote:
> >> On Fri, Sep 8, 2023 at 1:32 PM Jie Hai <haij...@huawei.com> wrote:
> >>>
> >>> The DPDK framework reports the queue state, which is stored in
> >>> dev->data->tx_queue_state and dev->data->rx_queue_state. The
> >>> state is maintained by the driver. Users may determine whether
> >>> a queue participates in packet forwarding based on the state,
> >>> for example,
> >>
> >> The driver is maintaining this state in dev_start / dev_stop and per
> >> queue start/stop handlers.
> >>
> >>>
> >>> [1] 5028f207a4fa ("app/testpmd: fix secondary process packet forwarding"
> >>> [2] 141a520b35f7 ("app/testpmd: fix primary process not polling all
> >>> queues")
> >>>
> >>> Therefore, the drivers need to modify the queue state in time
> >>> according to the actual situation, especially when dev_start
> >>> and dev_stop are called. see [3] for more information.
> >>>
> >>> [3]
> >>> https://inbox.dpdk.org/dev/20230721160422.3848154-1-ferruh.yi...@amd.com/
> >>>
> >>> This patchset also resubmit the patch [2] and makes some fixes on the
> >>> patch.
> >>
> >> I just had a quick look at some patches and I wonder if a better fix
> >> would be at the ethdev level, rather than fixing all drivers.
> >>
> >>
> >
> > I came here to make the same comment,
> >
> > Jie, I forgot if we discuss this already but,
> >
> > does it work if we update queue state in 'rte_eth_dev_start()' &
> > 'rte_eth_dev_stop()' when 'dev_start' & 'dev_stop' dev_ops succeeds?
> >
> > This solves the case driver forgets to update the queue state.
> >
> >
> Hi, Furrh and David,
>
> It's OK for dev_stop, but not enough for dev_start.
> Some drivers also support deferred_start.
> Therefore, not all queues are in the start state after dev_start.
>
> If we want to get that correct queue state at the framework level, I
> offer the following options:
>
> step 1. A capability(e.g. RTE_ETH_DEV_CAPA_DEFERRED_START) is added to
> the framework, indicating whether the driver supports deferred_start.
> The capability should be reported by the driver and user can get it by
> rte_eth_dev_info_get().
> step 2. All drivers that support deferred_start should report configuration
> information about deferred_start through
> rte_eth_rx_queue_info_get |rte_eth_tx_queue_info_get.
> step 3.The framework updates the queue status based on the support and
> configuration of deferred_start.
rte_eth_dev_start must only update the queue state if
rx_deferred_start is unset (see struct
rte_eth_rxconf::rx_deferred_start).
And the queue state needs to be updated in ethdev
rte_eth_dev_rx_queue_start/stop.
So I don't see where we need a new capability.
--
David Marchand
