Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

["Wang, Haiyue" <haiyue.wang@intel.com>]

NAK is expected. ☺ Just like the __rte_experimental API may be remove one day,
the application will fight for compiling, or just keep their own DPDK version.

Well, private handling is not a good thing, but HW is different, reserving some bits may
also be another API style. In real world, many workarounds exist, then a good solution
comes.

BR,
Haiyue

From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Saturday, June 22, 2019 00:46
To: Wang, Haiyue <haiyue.wang@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>; Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch



On Fri, Jun 21, 2019 at 6:37 PM Wang, Haiyue <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>> wrote:
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org<mailto:stephen@networkplumber.org>]
> Sent: Friday, June 21, 2019 23:14
> To: Wang, Haiyue <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com<mailto:arybchenko@solarflare.com>>; dev@dpdk.org<mailto:dev@dpdk.org>; Yigit, Ferruh <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>>;
> Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
>
> On Fri, 21 Jun 2019 07:43:13 +0000
> "Wang, Haiyue" <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>> wrote:
>
> > The experimental reserved bits number is 6 currently. Tt can be one-bit
> >
> > for each features up to the the maximum number 6. It can also be some
> >
> > bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> >
> >
> >
> > We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> >
> > unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> >
> > 39.
> >
> >
> >
> > This is not so nice for applications, they need to check PMD's driver
> >
> > name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> >
> > is good for the applications to make use of the hardware compatibility.
> >
> >
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>><mailto:haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>>
> >
> > I would say that it very bad for applications. It sounds like reserved bits
> > in registers which have meaning in fact and sometimes different meaning.
> > Of course, it is not that bad when rules are defined, but such kind of
> > features tend to spread and clutter up interfaces. IMHO, the feature is
> > really frightening.
>
> There are two issues. First, having more OFFLOAD capability feature bits
> is good. As long as these feature bits are well defined. If only one vendor
> implements that feature that is fine. Another vendor can implement the
> same thing, and application can know what it is asking for.
>
> The other issue is the limited number of feature bits. I expect that some
> time soon the bits will have to grow into an array and cause API/ABI
> break. That can be fixed when the last bit is exhausted.
>
>

If one bit for one feature, then it will be exhausted soon. That's why I said
using DEV_RX_OFFLOAD_PMD_SCRATCH bits to *encode* the PMD's offload if it is no
so common now, such as 6 bits will give the vendor 63 different types to select
their own features. And have 39 for common features defined in the future.

Frankly speaking, if we open some bits for PMD using, like the __rte_experimental
API style, then PMD will have more rich feature for open, customer can use the
experimental features, and these experimental features may be common in some day.

Just try to imagine what it would mean for the dataplane handling a packet:

if (is_port_vendor_X(portid)) {
  handle_exotic_vendor_X();
} else if (is_port_vendor_Y(portid)) {
  handle_exotic_vendor_Y();
} else {
  generic_handle();
}

Add to this changes with versions of dpdk since this would be out of the ABI/API stability and this is a huge mess.
This is a NAK for me.


--
David Marchand