[dpdk-dev,v2,1/2] mbuf: introduce new Tx offload flag for MPLS-in-UDP
Checks
Commit Message
From: Harish Patil <harish.patil@cavium.com>
Some PMDs need to know the tunnel type in order to handle advance TX
features. This patch adds a new TX offload flag for MPLS-in-UDP packets.
Signed-off-by: Harish Patil <harish.patil@cavium.com>
---
lib/librte_mbuf/rte_mbuf.c | 2 ++
lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++-------
2 files changed, 12 insertions(+), 7 deletions(-)
Comments
Hi Rasesh,
On Wed, 7 Jun 2017 00:43:48 -0700, Rasesh Mody <rasesh.mody@cavium.com> wrote:
> From: Harish Patil <harish.patil@cavium.com>
>
> Some PMDs need to know the tunnel type in order to handle advance TX
> features. This patch adds a new TX offload flag for MPLS-in-UDP packets.
>
> Signed-off-by: Harish Patil <harish.patil@cavium.com>
> ---
> lib/librte_mbuf/rte_mbuf.c | 2 ++
> lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++-------
> 2 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 0e3e36a..c2793fb 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -410,6 +410,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
> case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
> case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
> case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
> + case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
> default: return NULL;
> }
> }
> @@ -441,6 +442,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
> { PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
> "PKT_TX_TUNNEL_NONE" },
> { PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
> + { PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MPLSINUDP, NULL },
> };
> const char *name;
> unsigned int i;
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 1cb0310..27ad421 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -197,19 +197,22 @@
> * Offload the MACsec. This flag must be set by the application to enable
> * this offload feature for a packet to be transmitted.
> */
> -#define PKT_TX_MACSEC (1ULL << 44)
> +#define PKT_TX_MACSEC (1ULL << 43)
I'm not sure it is suitable to change the value of an existing
flag, since it breaks the ABI.
> /**
> - * Bits 45:48 used for the tunnel type.
> + * Bits 44:48 used for the tunnel type.
> * When doing Tx offload like TSO or checksum, the HW needs to configure the
> * tunnel type into the HW descriptors.
> */
> -#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
> -#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
> -#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
> -#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
> +/**< TX packet with MPLS-in-UDP RFC 7510 header. */
> +#define PKT_TX_TUNNEL_MPLSINUDP (0x1ULL << 44)
> +
> +#define PKT_TX_TUNNEL_VXLAN (0x2ULL << 44)
> +#define PKT_TX_TUNNEL_GRE (0x3ULL << 44)
> +#define PKT_TX_TUNNEL_IPIP (0x4ULL << 44)
> +#define PKT_TX_TUNNEL_GENEVE (0x5ULL << 45)
> /* add new TX TUNNEL type here */
> -#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
> +#define PKT_TX_TUNNEL_MASK (0x1FULL << 44)
>
> /**
> * Second VLAN insertion (QinQ) flag.
I dont understand why adding
#define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
wouldn't do the job?
Currently, the tunnel mask is 0xF << 45, which gives 16 possible values.
Regards,
Olivier
>Hi Rasesh,
>
>On Wed, 7 Jun 2017 00:43:48 -0700, Rasesh Mody <rasesh.mody@cavium.com>
>wrote:
>> From: Harish Patil <harish.patil@cavium.com>
>>
>> Some PMDs need to know the tunnel type in order to handle advance TX
>> features. This patch adds a new TX offload flag for MPLS-in-UDP packets.
>>
>> Signed-off-by: Harish Patil <harish.patil@cavium.com>
>> ---
>> lib/librte_mbuf/rte_mbuf.c | 2 ++
>> lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++-------
>> 2 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
>> index 0e3e36a..c2793fb 100644
>> --- a/lib/librte_mbuf/rte_mbuf.c
>> +++ b/lib/librte_mbuf/rte_mbuf.c
>> @@ -410,6 +410,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
>> case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
>> case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
>> case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
>> + case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
>> default: return NULL;
>> }
>> }
>> @@ -441,6 +442,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
>> { PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
>> "PKT_TX_TUNNEL_NONE" },
>> { PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
>> + { PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MPLSINUDP, NULL },
>> };
>> const char *name;
>> unsigned int i;
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> index 1cb0310..27ad421 100644
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -197,19 +197,22 @@
>> * Offload the MACsec. This flag must be set by the application to
>>enable
>> * this offload feature for a packet to be transmitted.
>> */
>> -#define PKT_TX_MACSEC (1ULL << 44)
>> +#define PKT_TX_MACSEC (1ULL << 43)
>
>I'm not sure it is suitable to change the value of an existing
>flag, since it breaks the ABI.
>
>
>> /**
>> - * Bits 45:48 used for the tunnel type.
>> + * Bits 44:48 used for the tunnel type.
>> * When doing Tx offload like TSO or checksum, the HW needs to
>>configure the
>> * tunnel type into the HW descriptors.
>> */
>> -#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
>> -#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
>> -#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
>> -#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
>> +/**< TX packet with MPLS-in-UDP RFC 7510 header. */
>> +#define PKT_TX_TUNNEL_MPLSINUDP (0x1ULL << 44)
>> +
>> +#define PKT_TX_TUNNEL_VXLAN (0x2ULL << 44)
>> +#define PKT_TX_TUNNEL_GRE (0x3ULL << 44)
>> +#define PKT_TX_TUNNEL_IPIP (0x4ULL << 44)
>> +#define PKT_TX_TUNNEL_GENEVE (0x5ULL << 45)
>> /* add new TX TUNNEL type here */
>> -#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
>> +#define PKT_TX_TUNNEL_MASK (0x1FULL << 44)
>>
>> /**
>> * Second VLAN insertion (QinQ) flag.
>
>I dont understand why adding
>#define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
>wouldn't do the job?
>
>Currently, the tunnel mask is 0xF << 45, which gives 16 possible values.
[Harish] Hi Olivier,
Not too sure whether I understand your comment.
My understanding is that those are bitmapped values for each Tx tunnel
type in the range [48:45].
They are not values. So defining PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
won’t work.
Currently bits[48:45] are reserved for Tx tunnel types. Bits[63:49] and
bit 44 are already taken.
Bits [43:18] are free. That’s why we see a code comment there:
/* add new RX flags here */
/* add new TX flags here */
So I could have added MPLSINUDP as:
#define PKT_TX_TUNNEL_MPLSINUDP (1ULL << 18)
But I wanted to group all Tx tunnel types together which is logical and
update PKT_TX_TUNNEL_MASK accordingly. So to accommodate the new MPLSoUDP
flag, I had to move PKT_TX_MACSEC to one bit position back from 44
to 43 and hence the code comment:
- * Bits 45:48 used for the tunnel type.
+ * Bits 44:48 used for the tunnel type.
But if this would cause a ABI breakage the option is to use bit 18 as
shown above and update the mask accordingly.
But the down side of this is that it will not be grouped together.
Please let me know if this is okay?
Thanks,
Harish
>
>Regards,
>Olivier
>
On Thu, 8 Jun 2017 21:46:00 +0000, "Patil, Harish" <Harish.Patil@cavium.com> wrote:
> >Hi Rasesh,
> >
> >On Wed, 7 Jun 2017 00:43:48 -0700, Rasesh Mody <rasesh.mody@cavium.com>
> >wrote:
> >> From: Harish Patil <harish.patil@cavium.com>
> >>
> >> Some PMDs need to know the tunnel type in order to handle advance TX
> >> features. This patch adds a new TX offload flag for MPLS-in-UDP packets.
> >>
> >> Signed-off-by: Harish Patil <harish.patil@cavium.com>
> >> ---
> >> lib/librte_mbuf/rte_mbuf.c | 2 ++
> >> lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++-------
> >> 2 files changed, 12 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> >> index 0e3e36a..c2793fb 100644
> >> --- a/lib/librte_mbuf/rte_mbuf.c
> >> +++ b/lib/librte_mbuf/rte_mbuf.c
> >> @@ -410,6 +410,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
> >> case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
> >> case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
> >> case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
> >> + case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
> >> default: return NULL;
> >> }
> >> }
> >> @@ -441,6 +442,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
> >> { PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
> >> "PKT_TX_TUNNEL_NONE" },
> >> { PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
> >> + { PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MPLSINUDP, NULL },
> >> };
> >> const char *name;
> >> unsigned int i;
> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> >> index 1cb0310..27ad421 100644
> >> --- a/lib/librte_mbuf/rte_mbuf.h
> >> +++ b/lib/librte_mbuf/rte_mbuf.h
> >> @@ -197,19 +197,22 @@
> >> * Offload the MACsec. This flag must be set by the application to
> >>enable
> >> * this offload feature for a packet to be transmitted.
> >> */
> >> -#define PKT_TX_MACSEC (1ULL << 44)
> >> +#define PKT_TX_MACSEC (1ULL << 43)
> >
> >I'm not sure it is suitable to change the value of an existing
> >flag, since it breaks the ABI.
> >
> >
> >> /**
> >> - * Bits 45:48 used for the tunnel type.
> >> + * Bits 44:48 used for the tunnel type.
> >> * When doing Tx offload like TSO or checksum, the HW needs to
> >>configure the
> >> * tunnel type into the HW descriptors.
> >> */
> >> -#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
> >> -#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
> >> -#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
> >> -#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
> >> +/**< TX packet with MPLS-in-UDP RFC 7510 header. */
> >> +#define PKT_TX_TUNNEL_MPLSINUDP (0x1ULL << 44)
> >> +
> >> +#define PKT_TX_TUNNEL_VXLAN (0x2ULL << 44)
> >> +#define PKT_TX_TUNNEL_GRE (0x3ULL << 44)
> >> +#define PKT_TX_TUNNEL_IPIP (0x4ULL << 44)
> >> +#define PKT_TX_TUNNEL_GENEVE (0x5ULL << 45)
> >> /* add new TX TUNNEL type here */
> >> -#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
> >> +#define PKT_TX_TUNNEL_MASK (0x1FULL << 44)
> >>
> >> /**
> >> * Second VLAN insertion (QinQ) flag.
> >
> >I dont understand why adding
> >#define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
> >wouldn't do the job?
> >
> >Currently, the tunnel mask is 0xF << 45, which gives 16 possible values.
>
> [Harish] Hi Olivier,
> Not too sure whether I understand your comment.
> My understanding is that those are bitmapped values for each Tx tunnel
> type in the range [48:45].
> They are not values. So defining PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
> won’t work.
Currently, we have:
#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
in binary: 000..000[0001]000..000
#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
in binary: 000..000[0010]000..000
#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
in binary: 000..000[0011]000..000
#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
in binary: 000..000[0100]000..000
So, I'm still saying there's a room for 11 more values.
Olivier
>
>On Thu, 8 Jun 2017 21:46:00 +0000, "Patil, Harish"
><Harish.Patil@cavium.com> wrote:
>> >Hi Rasesh,
>> >
>> >On Wed, 7 Jun 2017 00:43:48 -0700, Rasesh Mody <rasesh.mody@cavium.com>
>> >wrote:
>> >> From: Harish Patil <harish.patil@cavium.com>
>> >>
>> >> Some PMDs need to know the tunnel type in order to handle advance TX
>> >> features. This patch adds a new TX offload flag for MPLS-in-UDP
>>packets.
>> >>
>> >> Signed-off-by: Harish Patil <harish.patil@cavium.com>
>> >> ---
>> >> lib/librte_mbuf/rte_mbuf.c | 2 ++
>> >> lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++-------
>> >> 2 files changed, 12 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
>> >> index 0e3e36a..c2793fb 100644
>> >> --- a/lib/librte_mbuf/rte_mbuf.c
>> >> +++ b/lib/librte_mbuf/rte_mbuf.c
>> >> @@ -410,6 +410,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t
>>mask)
>> >> case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
>> >> case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
>> >> case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
>> >> + case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
>> >> default: return NULL;
>> >> }
>> >> }
>> >> @@ -441,6 +442,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t
>>mask)
>> >> { PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
>> >> "PKT_TX_TUNNEL_NONE" },
>> >> { PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
>> >> + { PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MPLSINUDP, NULL },
>> >> };
>> >> const char *name;
>> >> unsigned int i;
>> >> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
>> >> index 1cb0310..27ad421 100644
>> >> --- a/lib/librte_mbuf/rte_mbuf.h
>> >> +++ b/lib/librte_mbuf/rte_mbuf.h
>> >> @@ -197,19 +197,22 @@
>> >> * Offload the MACsec. This flag must be set by the application to
>> >>enable
>> >> * this offload feature for a packet to be transmitted.
>> >> */
>> >> -#define PKT_TX_MACSEC (1ULL << 44)
>> >> +#define PKT_TX_MACSEC (1ULL << 43)
>> >
>> >I'm not sure it is suitable to change the value of an existing
>> >flag, since it breaks the ABI.
>> >
>> >
>> >> /**
>> >> - * Bits 45:48 used for the tunnel type.
>> >> + * Bits 44:48 used for the tunnel type.
>> >> * When doing Tx offload like TSO or checksum, the HW needs to
>> >>configure the
>> >> * tunnel type into the HW descriptors.
>> >> */
>> >> -#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
>> >> -#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
>> >> -#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
>> >> -#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
>> >> +/**< TX packet with MPLS-in-UDP RFC 7510 header. */
>> >> +#define PKT_TX_TUNNEL_MPLSINUDP (0x1ULL << 44)
>> >> +
>> >> +#define PKT_TX_TUNNEL_VXLAN (0x2ULL << 44)
>> >> +#define PKT_TX_TUNNEL_GRE (0x3ULL << 44)
>> >> +#define PKT_TX_TUNNEL_IPIP (0x4ULL << 44)
>> >> +#define PKT_TX_TUNNEL_GENEVE (0x5ULL << 45)
>> >> /* add new TX TUNNEL type here */
>> >> -#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
>> >> +#define PKT_TX_TUNNEL_MASK (0x1FULL << 44)
>> >>
>> >> /**
>> >> * Second VLAN insertion (QinQ) flag.
>> >
>> >I dont understand why adding
>> >#define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
>> >wouldn't do the job?
>> >
>> >Currently, the tunnel mask is 0xF << 45, which gives 16 possible
>>values.
>>
>> [Harish] Hi Olivier,
>> Not too sure whether I understand your comment.
>> My understanding is that those are bitmapped values for each Tx tunnel
>> type in the range [48:45].
>> They are not values. So defining PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
>> won’t work.
>
>Currently, we have:
>
>#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
>in binary: 000..000[0001]000..000
>
>#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
>in binary: 000..000[0010]000..000
>
>#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
>in binary: 000..000[0011]000..000
>
>#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
>in binary: 000..000[0100]000..000
>
>So, I'm still saying there's a room for 11 more values.
>
>
>
>Olivier
>
[Harish] Okay thanks, got it. I shall send the v2 patch.
I have to update the driver also to use it as a value rather than as
bitmapped value.
@@ -410,6 +410,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
+ case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
default: return NULL;
}
}
@@ -441,6 +442,7 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
{ PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK,
"PKT_TX_TUNNEL_NONE" },
{ PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
+ { PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MPLSINUDP, NULL },
};
const char *name;
unsigned int i;
@@ -197,19 +197,22 @@
* Offload the MACsec. This flag must be set by the application to enable
* this offload feature for a packet to be transmitted.
*/
-#define PKT_TX_MACSEC (1ULL << 44)
+#define PKT_TX_MACSEC (1ULL << 43)
/**
- * Bits 45:48 used for the tunnel type.
+ * Bits 44:48 used for the tunnel type.
* When doing Tx offload like TSO or checksum, the HW needs to configure the
* tunnel type into the HW descriptors.
*/
-#define PKT_TX_TUNNEL_VXLAN (0x1ULL << 45)
-#define PKT_TX_TUNNEL_GRE (0x2ULL << 45)
-#define PKT_TX_TUNNEL_IPIP (0x3ULL << 45)
-#define PKT_TX_TUNNEL_GENEVE (0x4ULL << 45)
+/**< TX packet with MPLS-in-UDP RFC 7510 header. */
+#define PKT_TX_TUNNEL_MPLSINUDP (0x1ULL << 44)
+
+#define PKT_TX_TUNNEL_VXLAN (0x2ULL << 44)
+#define PKT_TX_TUNNEL_GRE (0x3ULL << 44)
+#define PKT_TX_TUNNEL_IPIP (0x4ULL << 44)
+#define PKT_TX_TUNNEL_GENEVE (0x5ULL << 45)
/* add new TX TUNNEL type here */
-#define PKT_TX_TUNNEL_MASK (0xFULL << 45)
+#define PKT_TX_TUNNEL_MASK (0x1FULL << 44)
/**
* Second VLAN insertion (QinQ) flag.