[PATCH] lib/net: added push MPLS header API

[PATCH] lib/net: added push MPLS header API

[Tanzeel-inline]

Push MPLS header after ethernet header, updates ethernet type and MPLS bs bit if required.

Signed-off-by: Tanzeel Ahmed <tanzeelahmed713@gmail.com>
---
 .mailmap           |  1 +
 lib/net/rte_mpls.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/.mailmap b/.mailmap
index 75884b6..5e06669 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1301,6 +1301,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 <tanzeelahmed713@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 3e8cb90..6ae72df 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_mbuf.h>
+#include <rte_ether.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -36,6 +38,63 @@ struct rte_mpls_hdr {
 	uint8_t  ttl;       /**< Time to live. */
 } __rte_packed;
 
+#define RTE_MPLS_HLEN 4 /**< Length of MPLS header. */
+
+/**
+ * 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 (If no ethernet header exists)		
+ */
+static inline int
+rte_mpls_push_over_l2(struct rte_mbuf **m, struct rte_mpls_hdr *mp) 
+{
+	struct rte_ether_hdr *oh, *nh;
+
+	/* 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);
+
+	mp->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);
+		mp->bs = 1;
+	}	
+	else
+	{
+		mp->bs = 0;
+	}
+
+	/* Copy the MPLS header after ethernet frame */
+	rte_memcpy(rte_pktmbuf_mtod_offset(*m, char*, sizeof(struct rte_ether_hdr)), mp, RTE_MPLS_HLEN);
+	
+	(*m)->data_len += RTE_MPLS_HLEN;
+	return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.3.1

Re: [PATCH] lib/net: added push MPLS header API

[Stephen Hemminger]

On Sat, 17 Dec 2022 12:59:57 -0500
Tanzeel-inline <tanxeel1.ahmed@gmail.com> wrote:


> +/**
> + * 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 (If no ethernet header exists)		
> + */

Should be marked experimental.

> +static inline int
> +rte_mpls_push_over_l2(struct rte_mbuf **m, struct rte_mpls_hdr *mp) 
trailing blank?
can mpls hdr be const?

> +{
> +	struct rte_ether_hdr *oh, *nh;
> +
> +	/* 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*/
add space after /* and before */

> +	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));

Not your problem, but having to double cast is only an artifact of
the poor design choice in DPDK for the return type of rte_pkmtmbuf_prepend()


> +	if (nh == NULL)
> +		return -ENOSPC;
> +
> +	memmove(nh, oh, RTE_ETHER_HDR_LEN);
> +
> +	mp->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))
> +	{

DPDK like kernel uses bracket "cuddle" style.

   if (conditon {
	...
   } else {
   }
> +		nh->ether_type = rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS);
> +		mp->bs = 1;
> +	}	
> +	else
> +	{
> +		mp->bs = 0;
> +	}
> +
> +	/* Copy the MPLS header after ethernet frame */
> +	rte_memcpy(rte_pktmbuf_mtod_offset(*m, char*, sizeof(struct rte_ether_hdr)), mp, RTE_MPLS_HLEN);

rte_memcpy is not preferred for small values.  Compiler can optimize memcpy() of small const sizes.
Might even be better to use structure assignment?

> +	
> +	(*m)->data_len += RTE_MPLS_HLEN;
> +	return 0;
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif


Side note: Why is DPDK version of checkpatch ignoring things that kernel checkpatch does not?

$ ~/kernel/linux//scripts/checkpatch.pl /tmp/mpls.mbox 
WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#102: 
Push MPLS header after ethernet header, updates ethernet type and MPLS bs bit if required.

ERROR: trailing whitespace
#143: FILE: lib/net/rte_mpls.h:45:
+ * $

ERROR: trailing whitespace
#147: FILE: lib/net/rte_mpls.h:49:
+ * $

ERROR: trailing whitespace
#153: FILE: lib/net/rte_mpls.h:55:
+ *   0 on success, -1 on error (If no ethernet header exists)^I^I$

ERROR: trailing whitespace
#156: FILE: lib/net/rte_mpls.h:58:
+rte_mpls_push_over_l2(struct rte_mbuf **m, struct rte_mpls_hdr *mp) $

ERROR: trailing whitespace
#177: FILE: lib/net/rte_mpls.h:79:
+^I$

ERROR: that open brace { should be on the previous line
#179: FILE: lib/net/rte_mpls.h:81:
+	if (nh->ether_type != rte_cpu_to_be_16(RTE_ETHER_TYPE_MPLS))
+	{

ERROR: trailing whitespace
#183: FILE: lib/net/rte_mpls.h:85:
+^I}^I$

ERROR: that open brace { should be on the previous line
#184: FILE: lib/net/rte_mpls.h:86:
+	else
+	{

ERROR: else should follow close brace '}'
#184: FILE: lib/net/rte_mpls.h:86:
+	}	
+	else

WARNING: line length of 104 exceeds 100 columns
#190: FILE: lib/net/rte_mpls.h:92:
+	rte_memcpy(rte_pktmbuf_mtod_offset(*m, char*, sizeof(struct rte_ether_hdr)), mp, RTE_MPLS_HLEN);

ERROR: trailing whitespace
#191: FILE: lib/net/rte_mpls.h:93:
+^I$

ERROR: Missing Signed-off-by: line by nominal patch author 'Tanzeel-inline <tanxeel1.ahmed@gmail.com>'

total: 11 errors, 2 warnings, 78 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/tmp/mpls.mbox has style problems, please review.

Also, current DPDK checkpatch has issue with mailmap.
This is not your problem. Looks like the change to check for mailmap
has chicken/egg problem

Tanzeel Ahmed <tanzeelahmed713@gmail.com> is unknown, please fix the commit message or update .mailmap.