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

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

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.
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
>