From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id BA9A8A00C4; Sat, 17 Dec 2022 20:46:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9C56740A7A; Sat, 17 Dec 2022 20:46:31 +0100 (CET) Received: from mail-pg1-f178.google.com (mail-pg1-f178.google.com [209.85.215.178]) by mails.dpdk.org (Postfix) with ESMTP id 68D4C40685 for ; Sat, 17 Dec 2022 20:46:30 +0100 (CET) Received: by mail-pg1-f178.google.com with SMTP id r18so3796812pgr.12 for ; Sat, 17 Dec 2022 11:46:30 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=gBxMRCTRbqx0BhSpWhBFDOUIVb5O4xmBVqniBbenga8=; b=pafCXrUueAF9iCBjEhoOnh3lRX4EnPNw7CLYHf24F89Kd6anrGHxCuwESOBfndUtw1 8mhjrNuKKYJacd8JIixRFm2fiiG4Q5Xv+09tqQQ9+q86qzZYlhGPf2yW6JlGIM4ozMml dOSFs2tN2aG7HN8KqrZZq9XMGAPtVlwdSqrDhqo2dmFpQLWFzgNJCk3jyrteXdx1todS M2V+OcDYoosqE0JKjJrmfJpG2J1bNMKGu+Skld4M5wYKNTIexA9SoT+4+TF31SggukGl gnQ+UR/rqEZVAUte+MMHI5qN/RcIBe6LS4BHIl+3AJs8pz8s9jA51+61n5hGD6mJ1uD6 K8lA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gBxMRCTRbqx0BhSpWhBFDOUIVb5O4xmBVqniBbenga8=; b=MQGTj45xHzVahW2jQ0QoF3QzCLUGy5pqRjrLH9VekMGPhXZBNeRaUK7+Yf2uiG2aQ0 Cjtoc6Ih1MMritOlCQ2wilJbIKjV/ymqzssR0Px+zJgT4Z61u0H1SfLPbN/9/tcYShBw hzMvmbX+IJQjNjMRH7hLSWGFpOxJNR4R0Baw3wJlRlqlS8R0jU8/sYiMTA7qfV6/bbXl XTuKIl1zpN1zh9op/6xu5kE8oLFBZcfd34aPIWiJY8ZThRQoqVf+YMR3h9fuDslR8jZm rv4T88DN8smFaYHPelwElJgrYPENiJbob6rsPaJWGQtAHKfcqmQZCCjgipb8wAmohz0I TZ0A== X-Gm-Message-State: AFqh2kqd3t6gK6a8En3Pofv43AIWRIfvGkrbE4jOqziuJU5tP3nJGB3K v+6UjaPb8HFdVdXVzW0QW4GULQ== X-Google-Smtp-Source: AMrXdXuVcKUYh2fX1eGJQT7XQN0JQ0LeeSKDqOlA74wK+j8Xk0ZI8zaw6vCpfcfsMHQAPPjKuWL8EQ== X-Received: by 2002:a62:38cf:0:b0:57e:866d:c090 with SMTP id f198-20020a6238cf000000b0057e866dc090mr2589050pfa.19.1671306389423; Sat, 17 Dec 2022 11:46:29 -0800 (PST) Received: from hermes.local (204-195-120-218.wavecable.com. [204.195.120.218]) by smtp.gmail.com with ESMTPSA id f4-20020aa79d84000000b00576e75e753asm3522024pfq.27.2022.12.17.11.46.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 17 Dec 2022 11:46:29 -0800 (PST) Date: Sat, 17 Dec 2022 11:46:27 -0800 From: Stephen Hemminger To: Tanzeel-inline Cc: olivier.matz@6wind.com, dev@dpdk.org, Tanzeel Ahmed Subject: Re: [PATCH] lib/net: added push MPLS header API Message-ID: <20221217114627.49cdb115@hermes.local> In-Reply-To: <1671299997-13001-1-git-send-email-tanxeel1.ahmed@gmail.com> References: <1671299997-13001-1-git-send-email-tanxeel1.ahmed@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Sat, 17 Dec 2022 12:59:57 -0500 Tanzeel-inline 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 ' 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 is unknown, please fix the commit message or update .mailmap.