DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v2] lib/net: add MPLS insert and strip functionality
@ 2023-02-19 22:43 Tanzeel-inline
  2023-02-24 11:25 ` [PATCH v3] " Tanzeel-inline
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Tanzeel-inline @ 2023-02-19 22:43 UTC (permalink / raw)
  To: olivier.matz, thomas; +Cc: dev, Tanzeel Ahmed

From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

This patch is new version of [PATCH] lib/net: added push MPLS header API.
I have also added the MPLS strip functionality in the same patch to
address the question asked in last patch.

> You should explain why you add this function.
None of the foundational NICs currently supports MPLS insertion and
stripping, this functionality can help users who rely on MPLS in their
network application.

> I'm not sure it should be inline
I did for perfomance in high-traffic application.

Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

---
v2:
* marked experimental
* coding style fixed
* changed rte_memcpy to memcpy
* mpls header marked as const in parameter
* added MPLS stripping functionality
---
 .mailmap           |  1 +
 lib/net/rte_mpls.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/.mailmap b/.mailmap
index a9f4f28..2af4e0d 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <t.yoshimura8869@gmail.com>
 Takuya Asada <syuu@cloudius-systems.com>
 Tal Avraham <talavr@annapurnalabs.com>
 Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
+Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
 Tao Y Yang <tao.y.yang@intel.com>
 Tao Zhu <taox.zhu@intel.com>
 Taripin Samuel <samuel.taripin@intel.com>
diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
index 51523e7..b51520f 100644
--- a/lib/net/rte_mpls.h
+++ b/lib/net/rte_mpls.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 #include <rte_byteorder.h>
+#include <rte_ether.h>
+#include <rte_mbuf.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,101 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Insert MPLS header into the packet.
+ * If it's first MPLS header to be inserted in the packet,
+ *  - Updates the ether type.
+ *  - Sets the MPLS bottom-of-stack bit to 1.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param mp
+ *   The pointer to the MPLS header.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static int
+rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
+{
+	struct rte_ether_hdr *oh, *nh;
+	struct rte_mpls_hdr *mph;
+
+	/* Can't insert header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
+		return -EINVAL;
+
+	/* Can't insert header if ethernet frame doesn't exist */
+	if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
+		return -EINVAL;
+
+	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
+	nh = (struct rte_ether_hdr *)(void *)
+		rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
+	if (nh == NULL)
+		return -ENOSPC;
+
+	memmove(nh, oh, RTE_ETHER_HDR_LEN);
+
+	/* Copy the MPLS header after ethernet frame */
+	mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
+			sizeof(struct rte_ether_hdr));
+	memcpy(mph, mp, RTE_MPLS_HLEN);
+
+	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
+
+	/* If first MPLS header, update ether type and bottom-of-stack bit */
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
+		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
+		mph->bs = 1;
+	} else {
+		mph->bs = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Strips MPLS from the packet. Doesn't update the ether type
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_strip_over_l2(struct rte_mbuf *m)
+{
+	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	struct rte_mpls_hdr *mph;
+	bool mpls_exist = true;
+
+	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+		return -1;
+
+	/* Stripping all MPLS header */
+	while (mpls_exist) {
+		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
+		sizeof(struct rte_ether_hdr));
+		if (mph->bs & 1)
+			mpls_exist = false;
+		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
+		eh, sizeof(struct rte_ether_hdr));
+		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	}
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] lib/net: add MPLS insert and strip functionality
  2023-02-19 22:43 [PATCH v2] lib/net: add MPLS insert and strip functionality Tanzeel-inline
@ 2023-02-24 11:25 ` Tanzeel-inline
  2023-02-24 16:29   ` Stephen Hemminger
  2023-02-25 13:53 ` [PATCH v4] " Tanzeel-inline
  2023-06-10 20:17 ` [PATCH v5] " Tanzeel-inline
  2 siblings, 1 reply; 12+ messages in thread
From: Tanzeel-inline @ 2023-02-24 11:25 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev, Tanzeel Ahmed

From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

This patch is new version of [PATCH] lib/net: added push MPLS header API.
I have also added the MPLS strip functionality to address the question
asked in last patch.

> You should explain why you add this function.
None of the foundational NICs currently supports MPLS insertion and
stripping, this functionality can help users who rely on MPLS in their
network application.

> I'm not sure it should be inline
I did for performance in high-traffic application.

Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

---
v3:
* fixed patch checks failure

v2:
* marked experimental
* coding style fixed
* changed rte_memcpy to memcpy
* mpls header marked as const in parameter
* added MPLS stripping functionality
---
 .mailmap           |  1 +
 lib/net/rte_mpls.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/.mailmap b/.mailmap
index a9f4f28..2af4e0d 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <t.yoshimura8869@gmail.com>
 Takuya Asada <syuu@cloudius-systems.com>
 Tal Avraham <talavr@annapurnalabs.com>
 Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
+Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
 Tao Y Yang <tao.y.yang@intel.com>
 Tao Zhu <taox.zhu@intel.com>
 Taripin Samuel <samuel.taripin@intel.com>
diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
index 51523e7..14b55fe 100644
--- a/lib/net/rte_mpls.h
+++ b/lib/net/rte_mpls.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 #include <rte_byteorder.h>
+#include <rte_ether.h>
+#include <rte_mbuf.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,101 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Insert MPLS header into the packet.
+ * If it's first MPLS header to be inserted in the packet,
+ *  - Updates the ether type.
+ *  - Sets the MPLS bottom-of-stack bit to 1.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param mp
+ *   The pointer to the MPLS header.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
+{
+	struct rte_ether_hdr *oh, *nh;
+	struct rte_mpls_hdr *mph;
+
+	/* Can't insert header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
+		return -EINVAL;
+
+	/* Can't insert header if ethernet frame doesn't exist */
+	if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
+		return -EINVAL;
+
+	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
+	nh = (struct rte_ether_hdr *)(void *)
+		rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
+	if (nh == NULL)
+		return -ENOSPC;
+
+	memmove(nh, oh, RTE_ETHER_HDR_LEN);
+
+	/* Copy the MPLS header after ethernet frame */
+	mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
+			sizeof(struct rte_ether_hdr));
+	memcpy(mph, mp, RTE_MPLS_HLEN);
+
+	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
+
+	/* If first MPLS header, update ether type and bottom-of-stack bit */
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
+		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
+		mph->bs = 1;
+	} else {
+		mph->bs = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Strips MPLS from the packet. Doesn't update the ether type
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_strip_over_l2(struct rte_mbuf *m)
+{
+	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	struct rte_mpls_hdr *mph;
+	bool mpls_exist = true;
+
+	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+		return -1;
+
+	/* Stripping all MPLS header */
+	while (mpls_exist) {
+		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
+		sizeof(struct rte_ether_hdr));
+		if (mph->bs & 1)
+			mpls_exist = false;
+		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
+		eh, sizeof(struct rte_ether_hdr));
+		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	}
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] lib/net: add MPLS insert and strip functionality
  2023-02-24 11:25 ` [PATCH v3] " Tanzeel-inline
@ 2023-02-24 16:29   ` Stephen Hemminger
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2023-02-24 16:29 UTC (permalink / raw)
  To: Tanzeel-inline; +Cc: olivier.matz, thomas, tanzeelahmed713, dev

On Fri, 24 Feb 2023 16:25:21 +0500
Tanzeel-inline <tanxeel1.ahmed@gmail.com> wrote:

> +	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
> +	nh = (struct rte_ether_hdr *)(void *)
> +		rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));

Don't need void * cast. Can cast result of prepend (char *) to ether header directly.

Note: rte_pktmbuf_append/prepend should have been written initially to
return void * to reduce the number of casts.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4] lib/net: add MPLS insert and strip functionality
  2023-02-19 22:43 [PATCH v2] lib/net: add MPLS insert and strip functionality Tanzeel-inline
  2023-02-24 11:25 ` [PATCH v3] " Tanzeel-inline
@ 2023-02-25 13:53 ` Tanzeel-inline
  2023-03-09 18:35   ` Tanzeel Ahmed
  2023-06-05  9:31   ` Olivier Matz
  2023-06-10 20:17 ` [PATCH v5] " Tanzeel-inline
  2 siblings, 2 replies; 12+ messages in thread
From: Tanzeel-inline @ 2023-02-25 13:53 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev, Tanzeel Ahmed

From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

This patch is new version of [PATCH] lib/net: added push MPLS header API.
I have also added the MPLS strip functionality to address the question
asked in last patch.

> You should explain why you add this function.
None of the foundational NICs currently supports MPLS insertion and
stripping, this functionality can help users who rely on MPLS in their
network application.

> I'm not sure it should be inline
I did for performance in high-traffic application.

Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

---
v4:
* Removed extra void cast.
* rte_pktmbuf_append/mtod now return void*.
  The memmove result is casted to rte_ether_hdr*.

v3:
* fixed patch check failure issue

v2:
* marked experimental
* coding style fixed
* changed rte_memcpy to memcpy
* mpls header marked as const in parameter
* added MPLS stripping functionality
---
 .mailmap           |  1 +
 lib/net/rte_mpls.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/.mailmap b/.mailmap
index a9f4f28..2af4e0d 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <t.yoshimura8869@gmail.com>
 Takuya Asada <syuu@cloudius-systems.com>
 Tal Avraham <talavr@annapurnalabs.com>
 Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
+Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
 Tao Y Yang <tao.y.yang@intel.com>
 Tao Zhu <taox.zhu@intel.com>
 Taripin Samuel <samuel.taripin@intel.com>
diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
index 51523e7..d7e267f 100644
--- a/lib/net/rte_mpls.h
+++ b/lib/net/rte_mpls.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 #include <rte_byteorder.h>
+#include <rte_ether.h>
+#include <rte_mbuf.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,101 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Insert MPLS header into the packet.
+ * If it's first MPLS header to be inserted in the packet,
+ *  - Updates the ether type.
+ *  - Sets the MPLS bottom-of-stack bit to 1.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param mp
+ *   The pointer to the MPLS header.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
+{
+	void *os, *ns;
+	struct rte_ether_hdr *nh;
+	struct rte_mpls_hdr *mph;
+
+	/* Can't insert header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
+		return -EINVAL;
+
+	/* Can't insert header if ethernet frame doesn't exist */
+	if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
+		return -EINVAL;
+
+	os = rte_pktmbuf_mtod(*m, void *);
+	ns = (void *)rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
+	if (ns == NULL)
+		return -ENOSPC;
+
+	nh = (struct rte_ether_hdr *)memmove(ns, os, RTE_ETHER_HDR_LEN);
+
+	/* Copy the MPLS header after ethernet frame */
+	mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
+			sizeof(struct rte_ether_hdr));
+	memcpy(mph, mp, RTE_MPLS_HLEN);
+
+	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
+
+	/* If first MPLS header, update ether type and bottom-of-stack bit */
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
+		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
+		mph->bs = 1;
+	} else {
+		mph->bs = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Strips MPLS from the packet. Doesn't update the ether type
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_strip_over_l2(struct rte_mbuf *m)
+{
+	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	struct rte_mpls_hdr *mph;
+	bool mpls_exist = true;
+
+	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+		return -1;
+
+	/* Stripping all MPLS header */
+	while (mpls_exist) {
+		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
+		sizeof(struct rte_ether_hdr));
+		if (mph->bs & 1)
+			mpls_exist = false;
+		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
+		eh, sizeof(struct rte_ether_hdr));
+		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	}
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] lib/net: add MPLS insert and strip functionality
  2023-02-25 13:53 ` [PATCH v4] " Tanzeel-inline
@ 2023-03-09 18:35   ` Tanzeel Ahmed
  2023-04-18 13:18     ` Tanzeel Ahmed
  2023-06-05  9:31   ` Olivier Matz
  1 sibling, 1 reply; 12+ messages in thread
From: Tanzeel Ahmed @ 2023-03-09 18:35 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev

[-- Attachment #1: Type: text/plain, Size: 5466 bytes --]

Any updates?


On Sat, Feb 25, 2023 at 6:53 PM Tanzeel-inline <tanxeel1.ahmed@gmail.com>
wrote:

> From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>
> This patch is new version of [PATCH] lib/net: added push MPLS header API.
> I have also added the MPLS strip functionality to address the question
> asked in last patch.
>
> > You should explain why you add this function.
> None of the foundational NICs currently supports MPLS insertion and
> stripping, this functionality can help users who rely on MPLS in their
> network application.
>
> > I'm not sure it should be inline
> I did for performance in high-traffic application.
>
> Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>
> ---
> v4:
> * Removed extra void cast.
> * rte_pktmbuf_append/mtod now return void*.
>   The memmove result is casted to rte_ether_hdr*.
>
> v3:
> * fixed patch check failure issue
>
> v2:
> * marked experimental
> * coding style fixed
> * changed rte_memcpy to memcpy
> * mpls header marked as const in parameter
> * added MPLS stripping functionality
> ---
>  .mailmap           |  1 +
>  lib/net/rte_mpls.h | 97
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
>
> diff --git a/.mailmap b/.mailmap
> index a9f4f28..2af4e0d 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <
> t.yoshimura8869@gmail.com>
>  Takuya Asada <syuu@cloudius-systems.com>
>  Tal Avraham <talavr@annapurnalabs.com>
>  Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
> +Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>  Tao Y Yang <tao.y.yang@intel.com>
>  Tao Zhu <taox.zhu@intel.com>
>  Taripin Samuel <samuel.taripin@intel.com>
> diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
> index 51523e7..d7e267f 100644
> --- a/lib/net/rte_mpls.h
> +++ b/lib/net/rte_mpls.h
> @@ -13,6 +13,8 @@
>
>  #include <stdint.h>
>  #include <rte_byteorder.h>
> +#include <rte_ether.h>
> +#include <rte_mbuf.h>
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -36,6 +38,101 @@ struct rte_mpls_hdr {
>         uint8_t  ttl;       /**< Time to live. */
>  } __rte_packed;
>
> +#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Insert MPLS header into the packet.
> + * If it's first MPLS header to be inserted in the packet,
> + *  - Updates the ether type.
> + *  - Sets the MPLS bottom-of-stack bit to 1.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param mp
> + *   The pointer to the MPLS header.
> + * @return
> + *   0 on success, -1 on error
> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
> +{
> +       void *os, *ns;
> +       struct rte_ether_hdr *nh;
> +       struct rte_mpls_hdr *mph;
> +
> +       /* Can't insert header if mbuf is shared */
> +       if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
> +               return -EINVAL;
> +
> +       /* Can't insert header if ethernet frame doesn't exist */
> +       if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
> +               return -EINVAL;
> +
> +       os = rte_pktmbuf_mtod(*m, void *);
> +       ns = (void *)rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
> +       if (ns == NULL)
> +               return -ENOSPC;
> +
> +       nh = (struct rte_ether_hdr *)memmove(ns, os, RTE_ETHER_HDR_LEN);
> +
> +       /* Copy the MPLS header after ethernet frame */
> +       mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
> +                       sizeof(struct rte_ether_hdr));
> +       memcpy(mph, mp, RTE_MPLS_HLEN);
> +
> +       mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
> +
> +       /* If first MPLS header, update ether type and bottom-of-stack bit
> */
> +       if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
> +               nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
> +               mph->bs = 1;
> +       } else {
> +               mph->bs = 0;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Strips MPLS from the packet. Doesn't update the ether type
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @return
> + *   0 on success, -1 on error
> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
> +{
> +       struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct
> rte_ether_hdr *);
> +       struct rte_mpls_hdr *mph;
> +       bool mpls_exist = true;
> +
> +       if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
> +               return -1;
> +
> +       /* Stripping all MPLS header */
> +       while (mpls_exist) {
> +               mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
> +               sizeof(struct rte_ether_hdr));
> +               if (mph->bs & 1)
> +                       mpls_exist = false;
> +               memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
> +               eh, sizeof(struct rte_ether_hdr));
> +               eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +       }
> +
> +       return 0;
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 1.8.3.1
>
>

[-- Attachment #2: Type: text/html, Size: 7290 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] lib/net: add MPLS insert and strip functionality
  2023-03-09 18:35   ` Tanzeel Ahmed
@ 2023-04-18 13:18     ` Tanzeel Ahmed
  2023-05-20 19:53       ` Tanzeel Ahmed
  0 siblings, 1 reply; 12+ messages in thread
From: Tanzeel Ahmed @ 2023-04-18 13:18 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev

[-- Attachment #1: Type: text/plain, Size: 5741 bytes --]

Ping.

On Thu, Mar 9, 2023 at 11:35 PM Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
wrote:

> Any updates?
>
>
> On Sat, Feb 25, 2023 at 6:53 PM Tanzeel-inline <tanxeel1.ahmed@gmail.com>
> wrote:
>
>> From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>
>> This patch is new version of [PATCH] lib/net: added push MPLS header API.
>> I have also added the MPLS strip functionality to address the question
>> asked in last patch.
>>
>> > You should explain why you add this function.
>> None of the foundational NICs currently supports MPLS insertion and
>> stripping, this functionality can help users who rely on MPLS in their
>> network application.
>>
>> > I'm not sure it should be inline
>> I did for performance in high-traffic application.
>>
>> Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>
>> ---
>> v4:
>> * Removed extra void cast.
>> * rte_pktmbuf_append/mtod now return void*.
>>   The memmove result is casted to rte_ether_hdr*.
>>
>> v3:
>> * fixed patch check failure issue
>>
>> v2:
>> * marked experimental
>> * coding style fixed
>> * changed rte_memcpy to memcpy
>> * mpls header marked as const in parameter
>> * added MPLS stripping functionality
>> ---
>>  .mailmap           |  1 +
>>  lib/net/rte_mpls.h | 97
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 98 insertions(+)
>>
>> diff --git a/.mailmap b/.mailmap
>> index a9f4f28..2af4e0d 100644
>> --- a/.mailmap
>> +++ b/.mailmap
>> @@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <
>> t.yoshimura8869@gmail.com>
>>  Takuya Asada <syuu@cloudius-systems.com>
>>  Tal Avraham <talavr@annapurnalabs.com>
>>  Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
>> +Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>  Tao Y Yang <tao.y.yang@intel.com>
>>  Tao Zhu <taox.zhu@intel.com>
>>  Taripin Samuel <samuel.taripin@intel.com>
>> diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
>> index 51523e7..d7e267f 100644
>> --- a/lib/net/rte_mpls.h
>> +++ b/lib/net/rte_mpls.h
>> @@ -13,6 +13,8 @@
>>
>>  #include <stdint.h>
>>  #include <rte_byteorder.h>
>> +#include <rte_ether.h>
>> +#include <rte_mbuf.h>
>>
>>  #ifdef __cplusplus
>>  extern "C" {
>> @@ -36,6 +38,101 @@ struct rte_mpls_hdr {
>>         uint8_t  ttl;       /**< Time to live. */
>>  } __rte_packed;
>>
>> +#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Insert MPLS header into the packet.
>> + * If it's first MPLS header to be inserted in the packet,
>> + *  - Updates the ether type.
>> + *  - Sets the MPLS bottom-of-stack bit to 1.
>> + *
>> + * @param m
>> + *   The pointer to the mbuf.
>> + * @param mp
>> + *   The pointer to the MPLS header.
>> + * @return
>> + *   0 on success, -1 on error
>> + */
>> +__rte_experimental
>> +static inline int
>> +rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
>> +{
>> +       void *os, *ns;
>> +       struct rte_ether_hdr *nh;
>> +       struct rte_mpls_hdr *mph;
>> +
>> +       /* Can't insert header if mbuf is shared */
>> +       if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
>> +               return -EINVAL;
>> +
>> +       /* Can't insert header if ethernet frame doesn't exist */
>> +       if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
>> +               return -EINVAL;
>> +
>> +       os = rte_pktmbuf_mtod(*m, void *);
>> +       ns = (void *)rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
>> +       if (ns == NULL)
>> +               return -ENOSPC;
>> +
>> +       nh = (struct rte_ether_hdr *)memmove(ns, os, RTE_ETHER_HDR_LEN);
>> +
>> +       /* Copy the MPLS header after ethernet frame */
>> +       mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
>> +                       sizeof(struct rte_ether_hdr));
>> +       memcpy(mph, mp, RTE_MPLS_HLEN);
>> +
>> +       mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
>> +
>> +       /* If first MPLS header, update ether type and bottom-of-stack
>> bit */
>> +       if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
>> +               nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
>> +               mph->bs = 1;
>> +       } else {
>> +               mph->bs = 0;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change without prior notice.
>> + *
>> + * Strips MPLS from the packet. Doesn't update the ether type
>> + *
>> + * @param m
>> + *   The pointer to the mbuf.
>> + * @return
>> + *   0 on success, -1 on error
>> + */
>> +__rte_experimental
>> +static inline int
>> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
>> +{
>> +       struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct
>> rte_ether_hdr *);
>> +       struct rte_mpls_hdr *mph;
>> +       bool mpls_exist = true;
>> +
>> +       if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
>> +               return -1;
>> +
>> +       /* Stripping all MPLS header */
>> +       while (mpls_exist) {
>> +               mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
>> +               sizeof(struct rte_ether_hdr));
>> +               if (mph->bs & 1)
>> +                       mpls_exist = false;
>> +               memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
>> +               eh, sizeof(struct rte_ether_hdr));
>> +               eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> --
>> 1.8.3.1
>>
>>

[-- Attachment #2: Type: text/html, Size: 7664 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] lib/net: add MPLS insert and strip functionality
  2023-04-18 13:18     ` Tanzeel Ahmed
@ 2023-05-20 19:53       ` Tanzeel Ahmed
  0 siblings, 0 replies; 12+ messages in thread
From: Tanzeel Ahmed @ 2023-05-20 19:53 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev

[-- Attachment #1: Type: text/plain, Size: 6037 bytes --]

Gentle Ping

On Tue, Apr 18, 2023 at 6:18 PM Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
wrote:

> Ping.
>
> On Thu, Mar 9, 2023 at 11:35 PM Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
> wrote:
>
>> Any updates?
>>
>>
>> On Sat, Feb 25, 2023 at 6:53 PM Tanzeel-inline <tanxeel1.ahmed@gmail.com>
>> wrote:
>>
>>> From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>>
>>> This patch is new version of [PATCH] lib/net: added push MPLS header API.
>>> I have also added the MPLS strip functionality to address the question
>>> asked in last patch.
>>>
>>> > You should explain why you add this function.
>>> None of the foundational NICs currently supports MPLS insertion and
>>> stripping, this functionality can help users who rely on MPLS in their
>>> network application.
>>>
>>> > I'm not sure it should be inline
>>> I did for performance in high-traffic application.
>>>
>>> Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>>
>>> ---
>>> v4:
>>> * Removed extra void cast.
>>> * rte_pktmbuf_append/mtod now return void*.
>>>   The memmove result is casted to rte_ether_hdr*.
>>>
>>> v3:
>>> * fixed patch check failure issue
>>>
>>> v2:
>>> * marked experimental
>>> * coding style fixed
>>> * changed rte_memcpy to memcpy
>>> * mpls header marked as const in parameter
>>> * added MPLS stripping functionality
>>> ---
>>>  .mailmap           |  1 +
>>>  lib/net/rte_mpls.h | 97
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 98 insertions(+)
>>>
>>> diff --git a/.mailmap b/.mailmap
>>> index a9f4f28..2af4e0d 100644
>>> --- a/.mailmap
>>> +++ b/.mailmap
>>> @@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <
>>> t.yoshimura8869@gmail.com>
>>>  Takuya Asada <syuu@cloudius-systems.com>
>>>  Tal Avraham <talavr@annapurnalabs.com>
>>>  Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
>>> +Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>>>  Tao Y Yang <tao.y.yang@intel.com>
>>>  Tao Zhu <taox.zhu@intel.com>
>>>  Taripin Samuel <samuel.taripin@intel.com>
>>> diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
>>> index 51523e7..d7e267f 100644
>>> --- a/lib/net/rte_mpls.h
>>> +++ b/lib/net/rte_mpls.h
>>> @@ -13,6 +13,8 @@
>>>
>>>  #include <stdint.h>
>>>  #include <rte_byteorder.h>
>>> +#include <rte_ether.h>
>>> +#include <rte_mbuf.h>
>>>
>>>  #ifdef __cplusplus
>>>  extern "C" {
>>> @@ -36,6 +38,101 @@ struct rte_mpls_hdr {
>>>         uint8_t  ttl;       /**< Time to live. */
>>>  } __rte_packed;
>>>
>>> +#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Insert MPLS header into the packet.
>>> + * If it's first MPLS header to be inserted in the packet,
>>> + *  - Updates the ether type.
>>> + *  - Sets the MPLS bottom-of-stack bit to 1.
>>> + *
>>> + * @param m
>>> + *   The pointer to the mbuf.
>>> + * @param mp
>>> + *   The pointer to the MPLS header.
>>> + * @return
>>> + *   0 on success, -1 on error
>>> + */
>>> +__rte_experimental
>>> +static inline int
>>> +rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr
>>> *mp)
>>> +{
>>> +       void *os, *ns;
>>> +       struct rte_ether_hdr *nh;
>>> +       struct rte_mpls_hdr *mph;
>>> +
>>> +       /* Can't insert header if mbuf is shared */
>>> +       if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
>>> +               return -EINVAL;
>>> +
>>> +       /* Can't insert header if ethernet frame doesn't exist */
>>> +       if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
>>> +               return -EINVAL;
>>> +
>>> +       os = rte_pktmbuf_mtod(*m, void *);
>>> +       ns = (void *)rte_pktmbuf_prepend(*m, sizeof(struct
>>> rte_mpls_hdr));
>>> +       if (ns == NULL)
>>> +               return -ENOSPC;
>>> +
>>> +       nh = (struct rte_ether_hdr *)memmove(ns, os, RTE_ETHER_HDR_LEN);
>>> +
>>> +       /* Copy the MPLS header after ethernet frame */
>>> +       mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
>>> +                       sizeof(struct rte_ether_hdr));
>>> +       memcpy(mph, mp, RTE_MPLS_HLEN);
>>> +
>>> +       mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
>>> +
>>> +       /* If first MPLS header, update ether type and bottom-of-stack
>>> bit */
>>> +       if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
>>> +               nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
>>> +               mph->bs = 1;
>>> +       } else {
>>> +               mph->bs = 0;
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>> +/**
>>> + * @warning
>>> + * @b EXPERIMENTAL: this API may change without prior notice.
>>> + *
>>> + * Strips MPLS from the packet. Doesn't update the ether type
>>> + *
>>> + * @param m
>>> + *   The pointer to the mbuf.
>>> + * @return
>>> + *   0 on success, -1 on error
>>> + */
>>> +__rte_experimental
>>> +static inline int
>>> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
>>> +{
>>> +       struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct
>>> rte_ether_hdr *);
>>> +       struct rte_mpls_hdr *mph;
>>> +       bool mpls_exist = true;
>>> +
>>> +       if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
>>> +               return -1;
>>> +
>>> +       /* Stripping all MPLS header */
>>> +       while (mpls_exist) {
>>> +               mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
>>> +               sizeof(struct rte_ether_hdr));
>>> +               if (mph->bs & 1)
>>> +                       mpls_exist = false;
>>> +               memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
>>> +               eh, sizeof(struct rte_ether_hdr));
>>> +               eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
>>> +       }
>>> +
>>> +       return 0;
>>> +}
>>> +
>>>  #ifdef __cplusplus
>>>  }
>>>  #endif
>>> --
>>> 1.8.3.1
>>>
>>>

[-- Attachment #2: Type: text/html, Size: 8060 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] lib/net: add MPLS insert and strip functionality
  2023-02-25 13:53 ` [PATCH v4] " Tanzeel-inline
  2023-03-09 18:35   ` Tanzeel Ahmed
@ 2023-06-05  9:31   ` Olivier Matz
  1 sibling, 0 replies; 12+ messages in thread
From: Olivier Matz @ 2023-06-05  9:31 UTC (permalink / raw)
  To: Tanzeel-inline; +Cc: thomas, tanzeelahmed713, dev

Hi,

Sorry for the late feedback, please see some comments below.

To be honest, I have some doubts about the usefulness of the patch,
especially the function that strips all the MPLS headers.

On Sat, Feb 25, 2023 at 06:53:43PM +0500, Tanzeel-inline wrote:
> From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
> 
> This patch is new version of [PATCH] lib/net: added push MPLS header API.
> I have also added the MPLS strip functionality to address the question
> asked in last patch.

You can remove this from commit log.
To add comments like these (which are indeed helpful), you can do it
after the '---', so it's not included in the commit log.

> 
> > You should explain why you add this function.

Please remove this line too.

> None of the foundational NICs currently supports MPLS insertion and
> stripping, this functionality can help users who rely on MPLS in their
> network application.
> 
> > I'm not sure it should be inline
> I did for performance in high-traffic application.

The inlining is probably discussable, but it is consistent with the
other equivalent functions in lib/net. Can you please reword the 2 lines
to remove the quote?

> 
> Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
> 
> ---
> v4:
> * Removed extra void cast.
> * rte_pktmbuf_append/mtod now return void*.
>   The memmove result is casted to rte_ether_hdr*.
> 
> v3:
> * fixed patch check failure issue
> 
> v2:
> * marked experimental
> * coding style fixed
> * changed rte_memcpy to memcpy
> * mpls header marked as const in parameter
> * added MPLS stripping functionality
> ---
>  .mailmap           |  1 +
>  lib/net/rte_mpls.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 98 insertions(+)
> 
> diff --git a/.mailmap b/.mailmap
> index a9f4f28..2af4e0d 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <t.yoshimura8869@gmail.com>
>  Takuya Asada <syuu@cloudius-systems.com>
>  Tal Avraham <talavr@annapurnalabs.com>
>  Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
> +Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>  Tao Y Yang <tao.y.yang@intel.com>
>  Tao Zhu <taox.zhu@intel.com>
>  Taripin Samuel <samuel.taripin@intel.com>
> diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
> index 51523e7..d7e267f 100644
> --- a/lib/net/rte_mpls.h
> +++ b/lib/net/rte_mpls.h
> @@ -13,6 +13,8 @@
>  
>  #include <stdint.h>
>  #include <rte_byteorder.h>
> +#include <rte_ether.h>
> +#include <rte_mbuf.h>
>  
>  #ifdef __cplusplus
>  extern "C" {
> @@ -36,6 +38,101 @@ struct rte_mpls_hdr {
>  	uint8_t  ttl;       /**< Time to live. */
>  } __rte_packed;
>  
> +#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Insert MPLS header into the packet.
> + * If it's first MPLS header to be inserted in the packet,
> + *  - Updates the ether type.
> + *  - Sets the MPLS bottom-of-stack bit to 1.

If it's first -> If it's the first

The first sentence uses the imperative form (which is preferred), so:
Updates -> Update
Sets -> Set

We can also document that the mbuf should not be shared, with enough
headroom.

> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param mp
> + *   The pointer to the MPLS header.
> + * @return
> + *   0 on success, -1 on error

final dot at the end

> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)

I don't see the need for **m, I think *m will do the job.

> +{
> +	void *os, *ns;
> +	struct rte_ether_hdr *nh;
> +	struct rte_mpls_hdr *mph;
> +
> +	/* Can't insert header if mbuf is shared */
> +	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
> +		return -EINVAL;
> +
> +	/* Can't insert header if ethernet frame doesn't exist */
> +	if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
> +		return -EINVAL;
> +
> +	os = rte_pktmbuf_mtod(*m, void *);

I prefer what you did in your previous revision (v3), without the *os
and *ns intermediate variables.

The first cast in (void *) is usually not needed in DPDK, but in your
case, it is better to add it because it is in a public header, and we
don't control the compiler warnings.

> +	ns = (void *)rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
> +	if (ns == NULL)
> +		return -ENOSPC;
> +
> +	nh = (struct rte_ether_hdr *)memmove(ns, os, RTE_ETHER_HDR_LEN);
> +
> +	/* Copy the MPLS header after ethernet frame */
> +	mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,

"struct rte_mpls_hdr*" -> "struct rte_mpls_hdr *"

> +			sizeof(struct rte_ether_hdr));
> +	memcpy(mph, mp, RTE_MPLS_HLEN);
> +
> +	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
> +
> +	/* If first MPLS header, update ether type and bottom-of-stack bit */
> +	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
> +		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
> +		mph->bs = 1;
> +	} else {
> +		mph->bs = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Strips MPLS from the packet. Doesn't update the ether type

-> Strip MPLS header from the packet, without updating the ether
   type.

I think the function should only strip the first MPLS header, it is
symmetric with the previous function, and more flexible.


> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @return
> + *   0 on success, -1 on error

final dot at the end

> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
> +{
> +	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +	struct rte_mpls_hdr *mph;
> +	bool mpls_exist = true;
> +

We should check the packet length, and that the packet is not shared.

> +	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
> +		return -1;
> +
> +	/* Stripping all MPLS header */
> +	while (mpls_exist) {
> +		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,

"struct rte_mpls_hdr*" -> "struct rte_mpls_hdr *"

> +		sizeof(struct rte_ether_hdr));

wrong indent

> +		if (mph->bs & 1)
> +			mpls_exist = false;
> +		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
> +		eh, sizeof(struct rte_ether_hdr));

wrong indent

technically, it is similar, but I think it is more readable to have
memove(), then rte_pktmbuf_adj(), on 2 distinct lines.

> +		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +	}
> +
> +	return 0;
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif
> -- 
> 1.8.3.1
> 

It would be great to have a unit test for these functions. This is even more
important because there is no user in the code (drivers or lib) that would
test it indirectly.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5] lib/net: add MPLS insert and strip functionality
  2023-02-19 22:43 [PATCH v2] lib/net: add MPLS insert and strip functionality Tanzeel-inline
  2023-02-24 11:25 ` [PATCH v3] " Tanzeel-inline
  2023-02-25 13:53 ` [PATCH v4] " Tanzeel-inline
@ 2023-06-10 20:17 ` Tanzeel-inline
  2023-06-22 19:47   ` Tanzeel Ahmed
  2 siblings, 1 reply; 12+ messages in thread
From: Tanzeel-inline @ 2023-06-10 20:17 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev, Tanzeel-inline

None of the foundational NICs currently supports MPLS insertion and
stripping, this functionality can help users who rely on MPLS in their
network application.

Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

---

This patch is new version of [PATCH] lib/net: added push MPLS header API.
I have also added the MPLS strip functionality to address the question
asked in last patch.

> To be honest, I have some doubts about the usefulness of the patch,
> especially the function that strips all the MPLS headers.

I believe it serves a practical purpose, in scenarios involving tapped traffic where MPLS headers
have not been stripped.
While some headers in the lib/net folder have well-defined functions, others are limited to their
structure alone. It would be advantageous to have basic functions available for all headers.

> I think the function should only strip the first MPLS header, it is
> symmetric with the previous function, and more flexible.

You are right, stripping one header is more flexible.
I updated the function to return 1, in case of stripping last MPLS header.

v5:
* Updated the MPLS strip function to strip one header at a time.
* Added the unit test cases.

v4:
* Removed extra void cast.
* rte_pktmbuf_append/mtod now return void*.
  The memmove result is casted to rte_ether_hdr*.

v3:
* fixed patch check failure issue

v2:
* marked experimental
* coding style fixed
* changed rte_memcpy to memcpy
* mpls header marked as const in parameter
* added MPLS stripping functionality
---
 app/test/meson.build |   2 +
 app/test/test_mpls.c | 180 +++++++++++++++++++++++++++++++++++++++++++
 lib/net/rte_mpls.h   | 106 +++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 app/test/test_mpls.c

diff --git a/app/test/meson.build b/app/test/meson.build
index f34d19e3c3..548349399f 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -95,6 +95,7 @@ test_sources = files(
         'test_meter.c',
         'test_mcslock.c',
         'test_mp_secondary.c',
+        'test_mpls.c',
         'test_per_lcore.c',
         'test_pflock.c',
         'test_pmd_perf.c',
@@ -205,6 +206,7 @@ fast_tests = [
         ['mempool_autotest', false, true],
         ['memzone_autotest', false, true],
         ['meter_autotest', true, true],
+        ['mpls_autotest', false, true],
         ['multiprocess_autotest', false, false],
         ['per_lcore_autotest', true, true],
         ['pflock_autotest', true, true],
diff --git a/app/test/test_mpls.c b/app/test/test_mpls.c
new file mode 100644
index 0000000000..8ff701f6e0
--- /dev/null
+++ b/app/test/test_mpls.c
@@ -0,0 +1,180 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ * Copyright(c) 2010-2014 Intel Corporation
+ */
+
+#include "test.h"
+
+#include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+
+#include <rte_common.h>
+#include <rte_memory.h>
+#include <rte_memcpy.h>
+#include <rte_mbuf.h>
+#include <rte_malloc.h>
+#include <rte_ether.h>
+#include <rte_mpls.h>
+
+#define MEMPOOL_CACHE_SIZE      32
+#define MBUF_DATA_SIZE          2048
+#define NB_MBUF                 128
+
+static int
+test_mpls_fail_push(struct rte_mbuf *m)
+{
+	struct rte_mpls_hdr mpls;
+
+	/* create dummy MPLS header */
+	mpls.tag_msb = 1;
+	mpls.tag_lsb = 2;
+	mpls.bs = 1;
+	mpls.tc = 1;
+	mpls.ttl = 255;
+
+	/* push first MPLS header */
+	if (rte_mpls_push_over_l2(m, &mpls) != 0)
+		return 0;
+	return -1;
+}
+
+static int
+test_mpls_push(struct rte_mbuf *m)
+{
+	struct rte_mpls_hdr mpls;
+
+	/* create dummy MPLS header */
+	mpls.tag_msb = 1;
+	mpls.tag_lsb = 2;
+	mpls.bs = 1;
+	mpls.tc = 1;
+	mpls.ttl = 255;
+
+	/* push first MPLS header */
+	if (rte_mpls_push_over_l2(m, &mpls) != 0) {
+		printf("Failed to insert mpls 1\n");
+		return -1;
+	}
+	if (rte_pktmbuf_pkt_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN) {
+		printf("Bad pkt length after inserting first mpls header\n");
+		return -1;
+	}
+
+	/* push second MPLS header*/
+	if (rte_mpls_push_over_l2(m, &mpls) != 0) {
+		printf("failed to insert mpls 1\n");
+		return -1;
+	}
+	if (rte_pktmbuf_pkt_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN * 2) {
+		printf("bad pkt length after inserting second mpls header\n");
+		return -1;
+	}
+	return 0;
+}
+
+static int
+test_mpls_fail_strip(struct rte_mbuf *m)
+{
+	/* strip MPLS headers */
+	if (rte_mpls_strip_over_l2(m) != 0)
+		return 0;
+	return -1;
+}
+
+static int
+test_mpls_strip(struct rte_mbuf *m)
+{
+	/* strip MPLS headers */
+	return rte_mpls_strip_over_l2(m);
+}
+
+static int
+test_mpls(void)
+{
+	int ret = -1;
+	struct rte_mempool *pktmbuf_pool = NULL;
+	struct rte_mbuf *m = NULL;
+	char *data;
+	struct rte_ether_hdr eh;
+
+	/* create pktmbuf pool */
+	pktmbuf_pool = rte_pktmbuf_pool_create("test_mpls_pool",
+			NB_MBUF, MEMPOOL_CACHE_SIZE, 0, MBUF_DATA_SIZE,
+			SOCKET_ID_ANY);
+
+	if (pktmbuf_pool == NULL) {
+		printf("cannot allocate mbuf pool\n");
+		goto err;
+	}
+
+    /* allocate mbuf from pool */
+	m = rte_pktmbuf_alloc(pktmbuf_pool);
+	if (m == NULL) {
+		printf("mbuf alloc failed\n");
+		goto err;
+	}
+	if (rte_pktmbuf_data_len(m) != 0) {
+		printf("mbuf alloc bad length\n");
+		goto err;
+	}
+
+	if (test_mpls_fail_push(m) < 0) {
+		printf("test_mpls_fail_push() failed\n");
+		goto err;
+	}
+
+	if (test_mpls_fail_strip(m) < 0) {
+		printf("test_mpls_fail_strip() failed\n");
+		goto err;
+	}
+
+	/* create a dummy ethernet header */
+	memset(&eh.src_addr, 0, RTE_ETHER_ADDR_LEN);
+	memset(&eh.dst_addr, 0, RTE_ETHER_ADDR_LEN);
+	eh.ether_type = rte_be_to_cpu_16(RTE_ETHER_TYPE_IPV4);
+
+	/* append ethernet header into mbuf */
+	data = rte_pktmbuf_append(m, RTE_ETHER_HDR_LEN);
+	if (data == NULL) {
+		printf("cannot append data\n");
+		goto err;
+	}
+	if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN) {
+		printf("bad pkt data length\n");
+		goto err;
+	}
+	memcpy(data, &eh, RTE_ETHER_HDR_LEN);
+
+	if (test_mpls_push(m) < 0) {
+		printf("test_mpls_push() failed\n");
+		goto err;
+	}
+
+	if (test_mpls_strip(m) < 0) {
+		printf("test_mpls_push() failed\n");
+		goto err;
+	}
+	if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN) {
+		printf("bad pkt data length after stripping first MPLS header\n");
+		goto err;
+	}
+
+	if (test_mpls_strip(m) < 0) {
+		printf("test_mpls_push() failed\n");
+		goto err;
+	}
+	if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN) {
+		printf("bad pkt data length after stripping second MPLS header\n");
+		goto err;
+	}
+	ret = 0;
+err:
+	if (m)
+		rte_pktmbuf_free(m);
+	if (pktmbuf_pool)
+		rte_mempool_free(pktmbuf_pool);
+	return ret;
+}
+
+REGISTER_TEST_COMMAND(mpls_autotest, test_mpls);
diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
index 3e8cb90ec3..a2072cdd10 100644
--- a/lib/net/rte_mpls.h
+++ b/lib/net/rte_mpls.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 #include <rte_byteorder.h>
+#include <rte_ether.h>
+#include <rte_mbuf.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,110 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Insert MPLS header into the packet.
+ * If it's the first MPLS header to be inserted in the packet,
+ *  - Update the ether type.
+ *  - Set the MPLS bottom-of-stack bit to 1.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param mp
+ *   The pointer to the MPLS header.
+ * @return
+ *   0 on success, -1 on error.
+ */
+__rte_experimental
+static inline int
+rte_mpls_push_over_l2(struct rte_mbuf *m, const struct rte_mpls_hdr *mp)
+{
+	struct rte_ether_hdr *oh, *nh;
+	struct rte_mpls_hdr *mph;
+
+	/* Can't insert header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(m) || rte_mbuf_refcnt_read(m) > 1)
+		return -EINVAL;
+
+	/* Can't insert header if ethernet frame doesn't exist */
+	if (rte_pktmbuf_data_len(m) < RTE_ETHER_HDR_LEN)
+		return -EINVAL;
+
+	oh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	nh = (struct rte_ether_hdr *)(void *)
+		rte_pktmbuf_prepend(m, sizeof(struct rte_mpls_hdr));
+	if (nh == NULL)
+		return -ENOSPC;
+
+	memmove(nh, oh, RTE_ETHER_HDR_LEN);
+
+	/* Copy the MPLS header after ethernet frame */
+	mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr *,
+			sizeof(struct rte_ether_hdr));
+	memcpy(mph, mp, RTE_MPLS_HLEN);
+
+	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
+
+	/* If first MPLS header, update ether type and bottom-of-stack bit */
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
+		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
+		mph->bs = 1;
+	} else {
+		mph->bs = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Strip MPLS header from the packet without updating the ether type.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @return
+ *   1 if last MPLS header is stripped,
+ *   0 on success,
+ *   -1 on error.
+ */
+__rte_experimental
+static inline int
+rte_mpls_strip_over_l2(struct rte_mbuf *m)
+{
+	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	struct rte_mpls_hdr *mph;
+	int result = 0;
+
+	/* Can't strip header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(m) || rte_mbuf_refcnt_read(m) > 1)
+		return -EINVAL;
+
+	/* Can't strip header if packet length is smaller */
+	if (rte_pktmbuf_data_len(m) < RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN)
+		return -EINVAL;
+
+	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+		return -1;
+
+	/* Stripping MPLS header */
+	mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr *,
+		sizeof(struct rte_ether_hdr));
+	if (mph->bs & 1)
+		result = 1;
+	memmove(
+		rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
+		eh, sizeof(struct rte_ether_hdr));
+	eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+
+	return result;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5] lib/net: add MPLS insert and strip functionality
  2023-06-10 20:17 ` [PATCH v5] " Tanzeel-inline
@ 2023-06-22 19:47   ` Tanzeel Ahmed
  0 siblings, 0 replies; 12+ messages in thread
From: Tanzeel Ahmed @ 2023-06-22 19:47 UTC (permalink / raw)
  To: Thomas Monjalon, Tanzeel Ahmed, olivier.matz; +Cc: dev

[-- Attachment #1: Type: text/plain, Size: 12150 bytes --]

Hi, @olivier.matz@6wind.com <olivier.matz@6wind.com>

On Sun, 11 Jun 2023, 1:17 am Tanzeel-inline, <tanxeel1.ahmed@gmail.com>
wrote:

> None of the foundational NICs currently supports MPLS insertion and
> stripping, this functionality can help users who rely on MPLS in their
> network application.
>
> Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
>
> ---
>
> This patch is new version of [PATCH] lib/net: added push MPLS header API.
> I have also added the MPLS strip functionality to address the question
> asked in last patch.
>
> > To be honest, I have some doubts about the usefulness of the patch,
> > especially the function that strips all the MPLS headers.
>
> I believe it serves a practical purpose, in scenarios involving tapped
> traffic where MPLS headers
> have not been stripped.
> While some headers in the lib/net folder have well-defined functions,
> others are limited to their
> structure alone. It would be advantageous to have basic functions
> available for all headers.
>
> > I think the function should only strip the first MPLS header, it is
> > symmetric with the previous function, and more flexible.
>
> You are right, stripping one header is more flexible.
> I updated the function to return 1, in case of stripping last MPLS header.
>
> v5:
> * Updated the MPLS strip function to strip one header at a time.
> * Added the unit test cases.
>
> v4:
> * Removed extra void cast.
> * rte_pktmbuf_append/mtod now return void*.
>   The memmove result is casted to rte_ether_hdr*.
>
> v3:
> * fixed patch check failure issue
>
> v2:
> * marked experimental
> * coding style fixed
> * changed rte_memcpy to memcpy
> * mpls header marked as const in parameter
> * added MPLS stripping functionality
> ---
>  app/test/meson.build |   2 +
>  app/test/test_mpls.c | 180 +++++++++++++++++++++++++++++++++++++++++++
>  lib/net/rte_mpls.h   | 106 +++++++++++++++++++++++++
>  3 files changed, 288 insertions(+)
>  create mode 100644 app/test/test_mpls.c
>
> diff --git a/app/test/meson.build b/app/test/meson.build
> index f34d19e3c3..548349399f 100644
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> @@ -95,6 +95,7 @@ test_sources = files(
>          'test_meter.c',
>          'test_mcslock.c',
>          'test_mp_secondary.c',
> +        'test_mpls.c',
>          'test_per_lcore.c',
>          'test_pflock.c',
>          'test_pmd_perf.c',
> @@ -205,6 +206,7 @@ fast_tests = [
>          ['mempool_autotest', false, true],
>          ['memzone_autotest', false, true],
>          ['meter_autotest', true, true],
> +        ['mpls_autotest', false, true],
>          ['multiprocess_autotest', false, false],
>          ['per_lcore_autotest', true, true],
>          ['pflock_autotest', true, true],
> diff --git a/app/test/test_mpls.c b/app/test/test_mpls.c
> new file mode 100644
> index 0000000000..8ff701f6e0
> --- /dev/null
> +++ b/app/test/test_mpls.c
> @@ -0,0 +1,180 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2010-2014 Intel Corporation
> + */
> +
> +#include "test.h"
> +
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +
> +#include <rte_common.h>
> +#include <rte_memory.h>
> +#include <rte_memcpy.h>
> +#include <rte_mbuf.h>
> +#include <rte_malloc.h>
> +#include <rte_ether.h>
> +#include <rte_mpls.h>
> +
> +#define MEMPOOL_CACHE_SIZE      32
> +#define MBUF_DATA_SIZE          2048
> +#define NB_MBUF                 128
> +
> +static int
> +test_mpls_fail_push(struct rte_mbuf *m)
> +{
> +       struct rte_mpls_hdr mpls;
> +
> +       /* create dummy MPLS header */
> +       mpls.tag_msb = 1;
> +       mpls.tag_lsb = 2;
> +       mpls.bs = 1;
> +       mpls.tc = 1;
> +       mpls.ttl = 255;
> +
> +       /* push first MPLS header */
> +       if (rte_mpls_push_over_l2(m, &mpls) != 0)
> +               return 0;
> +       return -1;
> +}
> +
> +static int
> +test_mpls_push(struct rte_mbuf *m)
> +{
> +       struct rte_mpls_hdr mpls;
> +
> +       /* create dummy MPLS header */
> +       mpls.tag_msb = 1;
> +       mpls.tag_lsb = 2;
> +       mpls.bs = 1;
> +       mpls.tc = 1;
> +       mpls.ttl = 255;
> +
> +       /* push first MPLS header */
> +       if (rte_mpls_push_over_l2(m, &mpls) != 0) {
> +               printf("Failed to insert mpls 1\n");
> +               return -1;
> +       }
> +       if (rte_pktmbuf_pkt_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN) {
> +               printf("Bad pkt length after inserting first mpls
> header\n");
> +               return -1;
> +       }
> +
> +       /* push second MPLS header*/
> +       if (rte_mpls_push_over_l2(m, &mpls) != 0) {
> +               printf("failed to insert mpls 1\n");
> +               return -1;
> +       }
> +       if (rte_pktmbuf_pkt_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN *
> 2) {
> +               printf("bad pkt length after inserting second mpls
> header\n");
> +               return -1;
> +       }
> +       return 0;
> +}
> +
> +static int
> +test_mpls_fail_strip(struct rte_mbuf *m)
> +{
> +       /* strip MPLS headers */
> +       if (rte_mpls_strip_over_l2(m) != 0)
> +               return 0;
> +       return -1;
> +}
> +
> +static int
> +test_mpls_strip(struct rte_mbuf *m)
> +{
> +       /* strip MPLS headers */
> +       return rte_mpls_strip_over_l2(m);
> +}
> +
> +static int
> +test_mpls(void)
> +{
> +       int ret = -1;
> +       struct rte_mempool *pktmbuf_pool = NULL;
> +       struct rte_mbuf *m = NULL;
> +       char *data;
> +       struct rte_ether_hdr eh;
> +
> +       /* create pktmbuf pool */
> +       pktmbuf_pool = rte_pktmbuf_pool_create("test_mpls_pool",
> +                       NB_MBUF, MEMPOOL_CACHE_SIZE, 0, MBUF_DATA_SIZE,
> +                       SOCKET_ID_ANY);
> +
> +       if (pktmbuf_pool == NULL) {
> +               printf("cannot allocate mbuf pool\n");
> +               goto err;
> +       }
> +
> +    /* allocate mbuf from pool */
> +       m = rte_pktmbuf_alloc(pktmbuf_pool);
> +       if (m == NULL) {
> +               printf("mbuf alloc failed\n");
> +               goto err;
> +       }
> +       if (rte_pktmbuf_data_len(m) != 0) {
> +               printf("mbuf alloc bad length\n");
> +               goto err;
> +       }
> +
> +       if (test_mpls_fail_push(m) < 0) {
> +               printf("test_mpls_fail_push() failed\n");
> +               goto err;
> +       }
> +
> +       if (test_mpls_fail_strip(m) < 0) {
> +               printf("test_mpls_fail_strip() failed\n");
> +               goto err;
> +       }
> +
> +       /* create a dummy ethernet header */
> +       memset(&eh.src_addr, 0, RTE_ETHER_ADDR_LEN);
> +       memset(&eh.dst_addr, 0, RTE_ETHER_ADDR_LEN);
> +       eh.ether_type = rte_be_to_cpu_16(RTE_ETHER_TYPE_IPV4);
> +
> +       /* append ethernet header into mbuf */
> +       data = rte_pktmbuf_append(m, RTE_ETHER_HDR_LEN);
> +       if (data == NULL) {
> +               printf("cannot append data\n");
> +               goto err;
> +       }
> +       if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN) {
> +               printf("bad pkt data length\n");
> +               goto err;
> +       }
> +       memcpy(data, &eh, RTE_ETHER_HDR_LEN);
> +
> +       if (test_mpls_push(m) < 0) {
> +               printf("test_mpls_push() failed\n");
> +               goto err;
> +       }
> +
> +       if (test_mpls_strip(m) < 0) {
> +               printf("test_mpls_push() failed\n");
> +               goto err;
> +       }
> +       if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN) {
> +               printf("bad pkt data length after stripping first MPLS
> header\n");
> +               goto err;
> +       }
> +
> +       if (test_mpls_strip(m) < 0) {
> +               printf("test_mpls_push() failed\n");
> +               goto err;
> +       }
> +       if (rte_pktmbuf_data_len(m) != RTE_ETHER_HDR_LEN) {
> +               printf("bad pkt data length after stripping second MPLS
> header\n");
> +               goto err;
> +       }
> +       ret = 0;
> +err:
> +       if (m)
> +               rte_pktmbuf_free(m);
> +       if (pktmbuf_pool)
> +               rte_mempool_free(pktmbuf_pool);
> +       return ret;
> +}
> +
> +REGISTER_TEST_COMMAND(mpls_autotest, test_mpls);
> diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
> index 3e8cb90ec3..a2072cdd10 100644
> --- a/lib/net/rte_mpls.h
> +++ b/lib/net/rte_mpls.h
> @@ -13,6 +13,8 @@
>
>  #include <stdint.h>
>  #include <rte_byteorder.h>
> +#include <rte_ether.h>
> +#include <rte_mbuf.h>
>
>  #ifdef __cplusplus
>  extern "C" {
> @@ -36,6 +38,110 @@ struct rte_mpls_hdr {
>         uint8_t  ttl;       /**< Time to live. */
>  } __rte_packed;
>
> +#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Insert MPLS header into the packet.
> + * If it's the first MPLS header to be inserted in the packet,
> + *  - Update the ether type.
> + *  - Set the MPLS bottom-of-stack bit to 1.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @param mp
> + *   The pointer to the MPLS header.
> + * @return
> + *   0 on success, -1 on error.
> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_push_over_l2(struct rte_mbuf *m, const struct rte_mpls_hdr *mp)
> +{
> +       struct rte_ether_hdr *oh, *nh;
> +       struct rte_mpls_hdr *mph;
> +
> +       /* Can't insert header if mbuf is shared */
> +       if (!RTE_MBUF_DIRECT(m) || rte_mbuf_refcnt_read(m) > 1)
> +               return -EINVAL;
> +
> +       /* Can't insert header if ethernet frame doesn't exist */
> +       if (rte_pktmbuf_data_len(m) < RTE_ETHER_HDR_LEN)
> +               return -EINVAL;
> +
> +       oh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +       nh = (struct rte_ether_hdr *)(void *)
> +               rte_pktmbuf_prepend(m, sizeof(struct rte_mpls_hdr));
> +       if (nh == NULL)
> +               return -ENOSPC;
> +
> +       memmove(nh, oh, RTE_ETHER_HDR_LEN);
> +
> +       /* Copy the MPLS header after ethernet frame */
> +       mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr *,
> +                       sizeof(struct rte_ether_hdr));
> +       memcpy(mph, mp, RTE_MPLS_HLEN);
> +
> +       mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
> +
> +       /* If first MPLS header, update ether type and bottom-of-stack bit
> */
> +       if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
> +               nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
> +               mph->bs = 1;
> +       } else {
> +               mph->bs = 0;
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice.
> + *
> + * Strip MPLS header from the packet without updating the ether type.
> + *
> + * @param m
> + *   The pointer to the mbuf.
> + * @return
> + *   1 if last MPLS header is stripped,
> + *   0 on success,
> + *   -1 on error.
> + */
> +__rte_experimental
> +static inline int
> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
> +{
> +       struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct
> rte_ether_hdr *);
> +       struct rte_mpls_hdr *mph;
> +       int result = 0;
> +
> +       /* Can't strip header if mbuf is shared */
> +       if (!RTE_MBUF_DIRECT(m) || rte_mbuf_refcnt_read(m) > 1)
> +               return -EINVAL;
> +
> +       /* Can't strip header if packet length is smaller */
> +       if (rte_pktmbuf_data_len(m) < RTE_ETHER_HDR_LEN + RTE_MPLS_HLEN)
> +               return -EINVAL;
> +
> +       if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
> +               return -1;
> +
> +       /* Stripping MPLS header */
> +       mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr *,
> +               sizeof(struct rte_ether_hdr));
> +       if (mph->bs & 1)
> +               result = 1;
> +       memmove(
> +               rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
> +               eh, sizeof(struct rte_ether_hdr));
> +       eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +
> +       return result;
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.34.1
>
>

[-- Attachment #2: Type: text/html, Size: 15681 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] lib/net: add MPLS insert and strip functionality
  2023-02-24 11:06 [PATCH v3] " Tanzeel-inline
@ 2024-10-03 18:46 ` Stephen Hemminger
  0 siblings, 0 replies; 12+ messages in thread
From: Stephen Hemminger @ 2024-10-03 18:46 UTC (permalink / raw)
  To: Tanzeel-inline; +Cc: olivier.matz, thomas, tanzeelahmed713, dev

On Fri, 24 Feb 2023 16:06:19 +0500
Tanzeel-inline <tanxeel1.ahmed@gmail.com> wrote:

> +static inline int
> +rte_mpls_strip_over_l2(struct rte_mbuf *m)
> +{
> +	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +	struct rte_mpls_hdr *mph;
> +	bool mpls_exist = true;
> +
> +	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
> +		return -1;
> +
> +	/* Stripping all MPLS header */
> +	while (mpls_exist) {
> +		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
space required (i.e "struct rte_mpls_hdr *")
> +		sizeof(struct rte_ether_hdr));
> +		if (mph->bs & 1)
> +			mpls_exist = false;
> +		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
> +		eh, sizeof(struct rte_ether_hdr));
> +		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
> +	}
> +
> +	return 0;
> +}

This function will fail for the case of segmented mbuf.
Also need to handle case where mbuf is corrupted and just has stack of mpls
tags and the last one is incomplete.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] lib/net: add MPLS insert and strip functionality
@ 2023-02-24 11:06 Tanzeel-inline
  2024-10-03 18:46 ` Stephen Hemminger
  0 siblings, 1 reply; 12+ messages in thread
From: Tanzeel-inline @ 2023-02-24 11:06 UTC (permalink / raw)
  To: olivier.matz, thomas, tanzeelahmed713; +Cc: dev, Tanzeel Ahmed

From: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

This patch is new version of [PATCH] lib/net: added push MPLS header API.
I have also added the MPLS strip functionality to address the question
asked in last patch.

> You should explain why you add this function.
None of the foundational NICs currently supports MPLS insertion and
stripping, this functionality can help users who rely on MPLS in their
network application.

> I'm not sure it should be inline
I did for performance in high-traffic application.

Signed-off-by: Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>

---
v3:
* fixed patch checks failure

v2:
* marked experimental
* coding style fixed
* changed rte_memcpy to memcpy
* mpls header marked as const in parameter
* added MPLS stripping functionality
---
 .mailmap           |  1 +
 lib/net/rte_mpls.h | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/.mailmap b/.mailmap
index a9f4f28..2af4e0d 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1312,6 +1312,7 @@ Takeshi Yoshimura <tyos@jp.ibm.com> <t.yoshimura8869@gmail.com>
 Takuya Asada <syuu@cloudius-systems.com>
 Tal Avraham <talavr@annapurnalabs.com>
 Tal Shnaiderman <talshn@nvidia.com> <talshn@mellanox.com>
+Tanzeel Ahmed <tanxeel1.ahmed@gmail.com>
 Tao Y Yang <tao.y.yang@intel.com>
 Tao Zhu <taox.zhu@intel.com>
 Taripin Samuel <samuel.taripin@intel.com>
diff --git a/lib/net/rte_mpls.h b/lib/net/rte_mpls.h
index 51523e7..14b55fe 100644
--- a/lib/net/rte_mpls.h
+++ b/lib/net/rte_mpls.h
@@ -13,6 +13,8 @@
 
 #include <stdint.h>
 #include <rte_byteorder.h>
+#include <rte_ether.h>
+#include <rte_mbuf.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,101 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Insert MPLS header into the packet.
+ * If it's first MPLS header to be inserted in the packet,
+ *  - Updates the ether type.
+ *  - Sets the MPLS bottom-of-stack bit to 1.
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @param mp
+ *   The pointer to the MPLS header.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_push_over_l2(struct rte_mbuf **m, const struct rte_mpls_hdr *mp)
+{
+	struct rte_ether_hdr *oh, *nh;
+	struct rte_mpls_hdr *mph;
+
+	/* Can't insert header if mbuf is shared */
+	if (!RTE_MBUF_DIRECT(*m) || rte_mbuf_refcnt_read(*m) > 1)
+		return -EINVAL;
+
+	/* Can't insert header if ethernet frame doesn't exist */
+	if (rte_pktmbuf_data_len(*m) < RTE_ETHER_HDR_LEN)
+		return -EINVAL;
+
+	oh = rte_pktmbuf_mtod(*m, struct rte_ether_hdr *);
+	nh = (struct rte_ether_hdr *)(void *)
+		rte_pktmbuf_prepend(*m, sizeof(struct rte_mpls_hdr));
+	if (nh == NULL)
+		return -ENOSPC;
+
+	memmove(nh, oh, RTE_ETHER_HDR_LEN);
+
+	/* Copy the MPLS header after ethernet frame */
+	mph = rte_pktmbuf_mtod_offset(*m, struct rte_mpls_hdr*,
+			sizeof(struct rte_ether_hdr));
+	memcpy(mph, mp, RTE_MPLS_HLEN);
+
+	mph->tag_msb = rte_cpu_to_be_16(mp->tag_msb);
+
+	/* If first MPLS header, update ether type and bottom-of-stack bit */
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS)) {
+		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
+		mph->bs = 1;
+	} else {
+		mph->bs = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice.
+ *
+ * Strips MPLS from the packet. Doesn't update the ether type
+ *
+ * @param m
+ *   The pointer to the mbuf.
+ * @return
+ *   0 on success, -1 on error
+ */
+__rte_experimental
+static inline int
+rte_mpls_strip_over_l2(struct rte_mbuf *m)
+{
+	struct rte_ether_hdr *eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	struct rte_mpls_hdr *mph;
+	bool mpls_exist = true;
+
+	if (eh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+		return -1;
+
+	/* Stripping all MPLS header */
+	while (mpls_exist) {
+		mph = rte_pktmbuf_mtod_offset(m, struct rte_mpls_hdr*,
+		sizeof(struct rte_ether_hdr));
+		if (mph->bs & 1)
+			mpls_exist = false;
+		memmove(rte_pktmbuf_adj(m, sizeof(struct rte_mpls_hdr)),
+		eh, sizeof(struct rte_ether_hdr));
+		eh = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
+	}
+
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-10-03 18:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-19 22:43 [PATCH v2] lib/net: add MPLS insert and strip functionality Tanzeel-inline
2023-02-24 11:25 ` [PATCH v3] " Tanzeel-inline
2023-02-24 16:29   ` Stephen Hemminger
2023-02-25 13:53 ` [PATCH v4] " Tanzeel-inline
2023-03-09 18:35   ` Tanzeel Ahmed
2023-04-18 13:18     ` Tanzeel Ahmed
2023-05-20 19:53       ` Tanzeel Ahmed
2023-06-05  9:31   ` Olivier Matz
2023-06-10 20:17 ` [PATCH v5] " Tanzeel-inline
2023-06-22 19:47   ` Tanzeel Ahmed
2023-02-24 11:06 [PATCH v3] " Tanzeel-inline
2024-10-03 18:46 ` Stephen Hemminger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).