[dpdk-dev,v3,1/4] mbuf:add three TX offload flags and change three fields
Commit Message
In place of removing the PKT_TX_VXLAN_CKSUM, we introduce 3 new flags: PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6 and PKT_TX_UDP_TUNNEL_PKT, and a new field: l4_tun_len.
Replace the inner_l2_len and the inner_l3_len field with the outer_l2_len and outer_l3_len field.
PKT_TX_OUTER_IP_CKSUM: is not used for non-tunnelling packet;hardware outer checksum for tunnelling packet.
PKT_TX_OUTER_IPV6: Tell the NIC it's an outer IPv6 packet for tunneling packet.
PKT_TX_UDP_TUNNEL_PKT: is used to tell PMD that the transmit packet is a UDP tunneling packet.
l4_tun_len: for VXLAN packet, it should be udp header length plus VXLAN header length.
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
---
lib/librte_mbuf/rte_mbuf.c | 6 +++++-
lib/librte_mbuf/rte_mbuf.h | 18 +++++++++++-------
2 files changed, 16 insertions(+), 8 deletions(-)
Comments
Hi Jijiang,
On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
> #define PKT_TX_IPV4 PKT_RX_IPV4_HDR
>
> /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
> #define PKT_TX_IPV6 PKT_RX_IPV6_HDR
The description still does not match what we discussed. Either we
have PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum
offload", or "packet is IPv4". I prefer the second one, but whatever
the choice is, the comments must be coherent.
> -#define PKT_TX_VLAN_PKT (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
> +/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
> +#define PKT_TX_OUTER_IP_CKSUM (1ULL << 58)
> +
> +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
> +#define PKT_TX_OUTER_IPV6 (1ULL << 59)
I think we should have the same flags with the same meanings for
inner and outer:
- PKT_TX_IPV4, PKT_TX_IP_CKSUM, PKT_TX_IPV6
- PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6
In your patch there is no PKT_TX_OUTER_IPV4 flag.
Regards,
Olivier
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Olivier MATZ
> Sent: Friday, November 28, 2014 9:37 AM
> To: Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
>
> Hi Jijiang,
>
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> > /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
> > #define PKT_TX_IPV4 PKT_RX_IPV4_HDR
> >
> > /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
> > #define PKT_TX_IPV6 PKT_RX_IPV6_HDR
>
> The description still does not match what we discussed. Either we
> have PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum
> offload", or "packet is IPv4". I prefer the second one, but whatever
> the choice is, the comments must be coherent.
>
> > -#define PKT_TX_VLAN_PKT (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
> > +/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
> > +#define PKT_TX_OUTER_IP_CKSUM (1ULL << 58)
> > +
> > +/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
> > +#define PKT_TX_OUTER_IPV6 (1ULL << 59)
>
> I think we should have the same flags with the same meanings for
> inner and outer:
>
> - PKT_TX_IPV4, PKT_TX_IP_CKSUM, PKT_TX_IPV6
> - PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IP_CKSUM, PKT_TX_OUTER_IPV6
Good point.
I still think: should we squeeze each of these triple in 2 bits?
Same as we doing for L4 checksum flags:
#define PKT_TX_IP_CKSUM (1ULL << X)
#define PKT_TX_IPV6 (2ULL << X)
#define PKT_TX_IPV4 (3ULL << X)
Same of outer flags.
Of course it means that they need to be mutually exclusive.
>
> In your patch there is no PKT_TX_OUTER_IPV4 flag.
>
> Regards,
> Olivier
Hi Olivier,
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Friday, November 28, 2014 5:37 PM
> To: Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and
> change three fields
>
> Hi Jijiang,
>
> On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> > /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO.
> */
> > #define PKT_TX_IPV4 PKT_RX_IPV4_HDR
> >
> > /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO.
> */
> > #define PKT_TX_IPV6 PKT_RX_IPV6_HDR
>
> The description still does not match what we discussed. Either we have
> PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum offload", or
> "packet is IPv4". I prefer the second one, but whatever the choice is, the
> comments must be coherent.
>
I agree.
"packet is IPv4" is ok for me, too.
The comment "Required for L4 checksum offload or TSO" is not added by me, I should have thought you added it during developing TSO.
Anyway, we came to an agreement for PKT_TX_IPV6/4 meaning, I will change the two flags comments.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Liu, Jijiang
> Sent: Friday, November 28, 2014 10:33 AM
> To: Olivier MATZ; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
>
> Hi Olivier,
>
> > -----Original Message-----
> > From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> > Sent: Friday, November 28, 2014 5:37 PM
> > To: Liu, Jijiang; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and
> > change three fields
> >
> > Hi Jijiang,
> >
> > On 11/27/2014 06:03 PM, Jijiang Liu wrote:
> > > /** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO.
> > */
> > > #define PKT_TX_IPV4 PKT_RX_IPV4_HDR
> > >
> > > /** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO.
> > */
> > > #define PKT_TX_IPV6 PKT_RX_IPV6_HDR
> >
> > The description still does not match what we discussed. Either we have
> > PKT_TX_IPV4 meaning "packet is IPv4 without requiring IP cksum offload", or
> > "packet is IPv4". I prefer the second one, but whatever the choice is, the
> > comments must be coherent.
> >
> I agree.
> "packet is IPv4" is ok for me, too.
> The comment "Required for L4 checksum offload or TSO" is not added by me, I should have thought you added it during developing
> TSO.
> Anyway, we came to an agreement for PKT_TX_IPV6/4 meaning, I will change the two flags comments.
>
>
Well, I still prefer them to be mutually exclusive.
Even better, if we can squeeze these 3 flags into 2 bits.
Would save us 2 bits, plus might be handy, as in the PMD you can do:
switch (ol_flags & TX_L3_MASK) {
case TX_IPV4:
...
break;
case TX_IPV6:
...
break;
case TX_IP_CKSUM:
...
break;
}
For the upper layer, I think there would be no big difference, what ways we will choose.
Konstantin
Hi Konstantin,
On 11/28/2014 11:40 AM, Ananyev, Konstantin wrote:
>
> Well, I still prefer them to be mutually exclusive.
> Even better, if we can squeeze these 3 flags into 2 bits.
> Would save us 2 bits, plus might be handy, as in the PMD you can do:
>
> switch (ol_flags & TX_L3_MASK) {
> case TX_IPV4:
> ...
> break;
> case TX_IPV6:
> ...
> break;
> case TX_IP_CKSUM:
> ...
> break;
> }
>
> For the upper layer, I think there would be no big difference, what ways we will choose.
I think the 2 informations are transversal, and that's why I would
prefer 2 flags. Also, having 2 separate flags would also help to keep
backward compatibility with previous versions.
It may help to have other points of view to make the good decision.
I'll follow the majority.
Regards,
Olivier
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Friday, November 28, 2014 11:00 AM
> To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
>
> Hi Konstantin,
>
> On 11/28/2014 11:40 AM, Ananyev, Konstantin wrote:
> >
> > Well, I still prefer them to be mutually exclusive.
> > Even better, if we can squeeze these 3 flags into 2 bits.
> > Would save us 2 bits, plus might be handy, as in the PMD you can do:
> >
> > switch (ol_flags & TX_L3_MASK) {
> > case TX_IPV4:
> > ...
> > break;
> > case TX_IPV6:
> > ...
> > break;
> > case TX_IP_CKSUM:
> > ...
> > break;
> > }
> >
> > For the upper layer, I think there would be no big difference, what ways we will choose.
>
> I think the 2 informations are transversal, and that's why I would
> prefer 2 flags. Also, having 2 separate flags would also help to keep
> backward compatibility with previous versions.
Hmm, not sure how we will break compatibility in that case?
If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current drivers nothing should change, no?
>
> It may help to have other points of view to make the good decision.
> I'll follow the majority.
>
> Regards,
> Olivier
Hi Konstantin,
On 11/28/2014 12:13 PM, Ananyev, Konstantin wrote:
>>> For the upper layer, I think there would be no big difference, what ways we will choose.
>>
>> I think the 2 informations are transversal, and that's why I would
>> prefer 2 flags. Also, having 2 separate flags would also help to keep
>> backward compatibility with previous versions.
>
> Hmm, not sure how we will break compatibility in that case?
> If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current drivers nothing should change, no?
Yes, you're right, sorry.
Olivier
> -----Original Message-----
> From: Olivier MATZ [mailto:olivier.matz@6wind.com]
> Sent: Friday, November 28, 2014 11:19 AM
> To: Ananyev, Konstantin; Liu, Jijiang; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/4] mbuf:add three TX offload flags and change three fields
>
> Hi Konstantin,
>
> On 11/28/2014 12:13 PM, Ananyev, Konstantin wrote:
> >>> For the upper layer, I think there would be no big difference, what ways we will choose.
> >>
> >> I think the 2 informations are transversal, and that's why I would
> >> prefer 2 flags. Also, having 2 separate flags would also help to keep
> >> backward compatibility with previous versions.
> >
> > Hmm, not sure how we will break compatibility in that case?
> > If we'll make TX_IP_CKSUM to be 1 bit value (1 << X) then for current drivers nothing should change, no?
>
> Yes, you're right, sorry.
Actually, you right - squeezing these 3 flags into 2 bits would break backward compatibility.
Sorry, didn't think properly.
Konstantin
>
> Olivier
@@ -240,7 +240,11 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
case PKT_TX_SCTP_CKSUM: return "PKT_TX_SCTP_CKSUM";
case PKT_TX_UDP_CKSUM: return "PKT_TX_UDP_CKSUM";
case PKT_TX_IEEE1588_TMST: return "PKT_TX_IEEE1588_TMST";
- case PKT_TX_VXLAN_CKSUM: return "PKT_TX_VXLAN_CKSUM";
+ case PKT_TX_UDP_TUNNEL_PKT: return "PKT_TX_UDP_TUNNEL_PKT";
+ case PKT_TX_IPV4: return "PKT_TX_IPV4";
+ case PKT_TX_IPV6: return "PKT_TX_IPV6";
+ case PKT_TX_OUTER_IP_CKSUM: return "PKT_TX_OUTER_IP_CKSUM";
+ case PKT_TX_OUTER_IPV6: return "PKT_TX_OUTER_IPV6";
case PKT_TX_TCP_SEG: return "PKT_TX_TCP_SEG";
default: return NULL;
}
@@ -99,10 +99,9 @@ extern "C" {
#define PKT_RX_TUNNEL_IPV6_HDR (1ULL << 12) /**< RX tunnel packet with IPv6 header. */
#define PKT_RX_FDIR_ID (1ULL << 13) /**< FD id reported if FDIR match. */
#define PKT_RX_FDIR_FLX (1ULL << 14) /**< Flexible bytes reported if FDIR match. */
-/* add new RX flags here */
/* add new TX flags here */
-#define PKT_TX_VXLAN_CKSUM (1ULL << 50) /**< TX checksum of VXLAN computed by NIC */
+#define PKT_TX_UDP_TUNNEL_PKT (1ULL << 50) /**< TX packet is an UDP tunneling packet */
#define PKT_TX_IEEE1588_TMST (1ULL << 51) /**< TX IEEE1588 packet to timestamp. */
/**
@@ -125,13 +124,19 @@ extern "C" {
#define PKT_TX_IP_CKSUM (1ULL << 54) /**< IP cksum of TX pkt. computed by NIC. */
#define PKT_TX_IPV4_CSUM PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */
+#define PKT_TX_VLAN_PKT (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
+
/** Tell the NIC it's an IPv4 packet. Required for L4 checksum offload or TSO. */
#define PKT_TX_IPV4 PKT_RX_IPV4_HDR
/** Tell the NIC it's an IPv6 packet. Required for L4 checksum offload or TSO. */
#define PKT_TX_IPV6 PKT_RX_IPV6_HDR
-#define PKT_TX_VLAN_PKT (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
+/** Outer IP cksum of TX pkt. computed by NIC for tunneling packet */
+#define PKT_TX_OUTER_IP_CKSUM (1ULL << 58)
+
+/** Tell the NIC it's an outer IPv6 packet for tunneling packet.*/
+#define PKT_TX_OUTER_IPV6 (1ULL << 59)
/**
* TCP segmentation offload. To enable this offload feature for a
@@ -266,10 +271,9 @@ struct rte_mbuf {
uint64_t tso_segsz:16; /**< TCP TSO segment size */
/* fields for TX offloading of tunnels */
- uint64_t inner_l3_len:9; /**< inner L3 (IP) Hdr Length. */
- uint64_t inner_l2_len:7; /**< inner L2 (MAC) Hdr Length. */
-
- /* uint64_t unused:8; */
+ uint64_t outer_l3_len:9; /**< outer L3 (IP) Hdr Length. */
+ uint64_t outer_l2_len:7; /**< outer L2 (MAC) Hdr Length. */
+ uint64_t l4_tun_len:8; /**< L4 tunnelling header length */
};
};
} __rte_cache_aligned;