On Sat, 17 Dec 2022 12:59:57 -0500
Tanzeel-inline <tanxeel1.ah...@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.ah...@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 <tanzeelahmed...@gmail.com> is unknown, please fix the commit
message or update .mailmap.
