Re: [PATCH v3 1/3] ethdev: add description for KEEP CRC offload

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 26 Nov 2024 15:47:32 +0800
Jie Hai <haijie1@huawei.com> wrote:

> Hi, Stephen Hemminger
> 
> Thanks for your review.
> I will add doc and fix on drivers in the next version.
> The test will be done later.
> 
> On 2024/11/23 1:10, Stephen Hemminger wrote:
> > On Fri, 19 Jul 2024 17:04:13 +0800
> > Jie Hai <haijie1@huawei.com> wrote:
> >   
> >> From: Dengdui Huang <huangdengdui@huawei.com>
> >>
> >> The data exceeds the pkt_len in mbuf is inavailable for user.  
> >                                            unavailable
> >   
> >> When KEEP CRC offload is enabled, CRC field length should be
> >> included in the pkt_len in mbuf. However, almost of drivers
> >> supported KEEP CRC feature didn't add the CRC data length to
> >> pkt_len. So it is very necessary to add comments for this.  
> > 
> > All drivers must do the same thing, or this is a serious bug
> > in the drivers. Just changing a comment is not going to be helpful.
> > 
> > To fix this right:
> >   1. Do a test with one of the original drivers in DPDK that has this
> >      feature. I would suggest ixgbe, mlx5 or bnxt.
> > I can test it on ixgbe and mlx5.
> >   2. Add a test to the PMD tests that validates this (if there is not
> >      one already).
> >   
> Maybe later and not come with this patchset.
> >   3. Put the documentation in a place where it shows up in user documentation.
> >      Either in doxygen comment or in doc/guides/nics
> >   
> Will add in the next version.
> >   4. Verify that all devices conform to the desired behavior
> > 
> > I can help, but only have some old mlx5 cards to test here.  
> Thanks.
> > Just putting comment in ethdev.h is not enough.
> >   
> 
> Thanks,
> Jie Hai
> 
> >   

After more discussion, decided to not include this patch (only the two bugfixes).
The proper place to put it is in the programmer's guide, and also you have
discovered a messy place in the driver API. In 10 minutes, already found several
issues in other drivers around this. Like checking wrong bits, dead code, etc.