On Tue, Aug 22, 2023 at 9:33 AM Zhang, Qi Z <qi.z.zh...@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. -- David Marchand