[PATCH] net/iavf: fix checksum offloading

[Zhang, Qi Z]

> -----Original Message-----
> From: David Marchand <david.marchand at redhat.com>
> Sent: Tuesday, August 22, 2023 3:40 PM
> To: Zhang, Qi Z <qi.z.zhang at intel.com>
> Cc: dev at dpdk.org; echaudro at redhat.com; mkp at redhat.com;
> stable at dpdk.org; Wu, Jingjing <jingjing.wu at intel.com>; Xing, Beilei
> <beilei.xing at intel.com>; Doherty, Declan <declan.doherty at intel.com>; Sinha,
> Abhijit <abhijit.sinha at intel.com>; Nicolau, Radu <radu.nicolau at 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 at 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