RE: [PATCH] net/iavf: fix checksum offloading

["Zhang, Qi Z" <qi.z.zhang@intel.com>]

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, August 22, 2023 3:40 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; echaudro@redhat.com; mkp@redhat.com;
> stable@dpdk.org; Wu, Jingjing <jingjing.wu@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Doherty, Declan <declan.doherty@intel.com>; Sinha,
> Abhijit <abhijit.sinha@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>
> Subject: Re: [PATCH] net/iavf: fix checksum offloading
> 
> On Tue, Aug 22, 2023 at 9:33 AM Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> > > If the driver reads l2_len or l3_len, this is an undefined behavior:
> > > for example, OVS might have been using l2_len or l3_len for its
> > > internal uses (though I agree it would be risky for an application to do so).
> > >
> > > We probably need to fix access to l2_len a few lines before my patch.
> > >
> > >         if (m->ol_flags & RTE_MBUF_F_TX_TUNNEL_MASK &&
> > >                         !(m->ol_flags & RTE_MBUF_F_TX_SEC_OFFLOAD))
> > >                 offset |= (m->outer_l2_len >> 1)
> > >                         << IAVF_TX_DESC_LENGTH_MACLEN_SHIFT;
> > >         else
> > >                 offset |= (m->l2_len >> 1)
> > >                         << IAVF_TX_DESC_LENGTH_MACLEN_SHIFT;
> > >
> > >
> > > But to be clear, no I don't think looking at l3_len value is better:
> > > it should not be read at all.
> >
> > Yes, you may be correct; it appears that this issue is unrelated to l3_len. The
> primary concern is to prevent the configuration of Tx descriptors with incorrect
> values.
> >
> > Based on your description, it seems the problem arises when the PMD sets
> MACLEN to 0 and also configures IIPT as 01b, Is this correct?
> >
> > To prevent this issue, we could implement a check where, if l2_len is
> > 0, we simply ignore the IIPT configuration and keep it at 0. (which
> > leads to same configure with your patch)
> 
> The driver is _not_ supposed to read l2_len or l3_len if no valid ol_flags is set.
> I will reject any suggestion where you consider their values.

Alright, we could directly verify the MACLEN value in offset and decide if IIPT should be configured or not, regardless of the values of l2_len and l3_len
And before that, l2_len / l3_len should not be accessed by the driver if no related offload is required.

> 
> 
> --
> David Marchand