Re: [dpdk-dev] [PATCH v3 1/5] ethdev: add API to negotiate delivery of Rx meta data

[Ori Kam <orika@nvidia.com>]

Hi

> -----Original Message-----
> From: Ivan Malov <Ivan.Malov@oktetlabs.ru>
> Subject: Re: [dpdk-dev] [PATCH v3 1/5] ethdev: add API to negotiate delivery
> of Rx meta data
> 
> 
> 
> On 01/10/2021 01:12, Ajit Khaparde wrote:
> > On Thu, Sep 30, 2021 at 3:01 PM Ivan Malov <Ivan.Malov@oktetlabs.ru>
> wrote:
> >>
> >> Hi Ajit,
> >>
> >> On 01/10/2021 00:48, Ajit Khaparde wrote:
> >>> ::::
> >>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>>> 97ae52e17e..7a8da3d7ab 100644
> >>>> --- a/app/test-pmd/testpmd.c
> >>>> +++ b/app/test-pmd/testpmd.c
> >>>> @@ -1485,10 +1485,36 @@ static void
> >>>>    init_config_port_offloads(portid_t pid, uint32_t socket_id)
> >>>>    {
> >>>>           struct rte_port *port = &ports[pid];
> >>>> +       uint64_t rx_meta_features = 0;
> >>>>           uint16_t data_size;
> >>>>           int ret;
> >>>>           int i;
> >>>>
> >>>> +       rx_meta_features |= RTE_ETH_RX_META_USER_FLAG;
> >>>> +       rx_meta_features |= RTE_ETH_RX_META_USER_MARK;
> >>>> +       rx_meta_features |= RTE_ETH_RX_META_TUNNEL_ID;
> >>>> +
> >>>> +       ret = rte_eth_rx_meta_negotiate(pid, &rx_meta_features);
> >>>> +       if (ret == 0) {
> >>>> +               if (!(rx_meta_features & RTE_ETH_RX_META_USER_FLAG)) {
> >>>> +                       TESTPMD_LOG(INFO, "Flow action FLAG will
> >>>> + not affect Rx mbufs on port %u\n",
> >>> Log level info might be a little too noisy?
> >>
> >> Do you really think so? But main() sets default log level to DEBUG, quote:
> >>       rte_log_set_level(testpmd_logtype, RTE_LOG_DEBUG);
> >>
> >> If I go for DEBUG instead of INFO here, it won't get any quieter, will it?
> > You are right. It won't.
> > But then three extra messages per port will stand out. But that's my
> opinion.
> > Maybe you could log the message when a flow is created with any of the
> > meta features?
> 
> The idea is to warn the user from the very beginning that certain flow
> primitives won't actually work. This way, the user can refrain from trying to
> use them in flow rules. This might save their time.
> 
> But I don't mind going for DEBUG here. More opinions are welcome.
> 

+1 for doing it only for configuration, and not per flow.

> >
> >>
> >>>
> >>>> +                                   pid);
> >>>> +               }
> >>>> +
> >>>> +               if (!(rx_meta_features & RTE_ETH_RX_META_USER_MARK)) {
> >>>> +                       TESTPMD_LOG(INFO, "Flow action MARK will not affect Rx
> mbufs on port %u\n",
> >>>> +                                   pid);
> >>>> +               }
> >>>> +
> >>>> +               if (!(rx_meta_features & RTE_ETH_RX_META_TUNNEL_ID)) {
> >>>> +                       TESTPMD_LOG(INFO, "Flow tunnel offload support might
> be limited or unavailable on port %u\n",
> >>>> +                                   pid);
> >>>> +               }
> >>> :::
> >>>>
> >>
> >> --
> >> Ivan M
> 
> --
> Ivan M
Best,
Ori