Re: [dpdk-dev] [PATCH v3 08/13] testpmd: rework csum forward engine

["Liu, Jijiang" <jijiang.liu@intel.com>]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, November 27, 2014 12:35 AM
> To: Olivier MATZ; dev@dpdk.org
> Cc: Walukiewicz, Miroslaw; Liu, Jijiang; Liu, Yong; jigsaw@gmail.com; Richardson,
> Bruce
> Subject: RE: [PATCH v3 08/13] testpmd: rework csum forward engine
> 
> Hi Oliver,
> 
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Wednesday, November 26, 2014 2:55 PM
> > To: Ananyev, Konstantin; dev@dpdk.org
> > Cc: Walukiewicz, Miroslaw; Liu, Jijiang; Liu, Yong; jigsaw@gmail.com;
> > Richardson, Bruce
> > Subject: Re: [PATCH v3 08/13] testpmd: rework csum forward engine
> >
> > Hi Konstantin,
> >
> > On 11/26/2014 01:25 PM, Ananyev, Konstantin wrote:
> > >> By the way (this is probably off-topic), but I'm wondering if the
> > >> TX flags should have the same values than the RX flags:
> > >>
> > >>     #define PKT_TX_IPV4          PKT_RX_IPV4_HDR
> > >>     #define PKT_TX_IPV6          PKT_RX_IPV6_HDR
> > >
> > > Thought about that too.
> > >  From one side,  it is a bit out of our concept: separate RX and TX falgs.
> > >  From other side, it allows us to save 2 bits in the ol_flags.
> > > Don't have any strong opinion here.
> > > What do you think?
> >
> > I have no strong opinion too, but I have a preference for 2 different
> > bit values for these flags:
> >
> > - as you say, it's matches the concept (RX and TX flags are separated)
> >
> > - 64 bits is a lot, we have some time before there is no more available
> >    bit... and I hope we it will never occur because it would become
> >    complex for an application to handle them all
> >
> > - it will avoid to send a packet with a bad info:
> >    - we receive a Ether/IP6/IP4/L4/data packet
> >    - the driver sets PKT_RX_IPV6_HDR
> >    - the stack decapsulates IP6
> >    - the stack sends the packet, it has the PKT_TX_IPV6 flag but it's an
> >      IPv4 packet
> 
> Ah yes, you right, if we keep them the same, then upper layer always has to clear
> PKT_RX_IPV4_HDR |  PKT_RX_IPV6_HDR before setting TX offload flags and
> passing packet to the PMD for TX.
> And if the upper layer wouldn't do that - it might cause a problem.
> With your example above - if at last step the stack sets  PKT_TX_IP_CKSUM for
> the packet, then PMD will receive an mbuf with (PKT_TX_IPV6 |
> PKT_TX_IP_CKSUM) set.
> Though from PKT_TX_IP_CKSUM/ PKT_TX_IPV6/ PKT_TX_IPV4 only one can be
> set, as they are mutually exclusive.
> So i40e PMD will get confused and might not be able to arm TX descriptor
> propely.
> So yes, we need to make them a proper TX flags.

I have changed definitions of the two flags in the patch of rework VXLAN TX checksum.

> Thanks for spotting it.
> Konstantin
> 
> >
> >    This is not a real problem as the flag will not be used by the
> >    driver/hardware (it's only mandatory for hw cksum / tso), but
> >    it can be confusing.
> >
> > Regards,
> > Olivier
> >