From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH v2 1/2] mbuf: introduce new Tx offload flag for MPLS-in-UDP Date: Fri, 9 Jun 2017 09:18:11 +0200 Message-ID: <20170609091811.0867b1d1@platinum> References: <1495960654-352-1-git-send-email-rasesh.mody@cavium.com> <1496821429-6954-1-git-send-email-rasesh.mody@cavium.com> <20170608142545.10cc8326@platinum> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: "Mody, Rasesh" , Ferruh Yigit , "dev@dpdk.org" , Dept-Eng DPDK Dev To: "Patil, Harish" Return-path: Received: from mail-wr0-f175.google.com (mail-wr0-f175.google.com [209.85.128.175]) by dpdk.org (Postfix) with ESMTP id 2FFD22BA3 for ; Fri, 9 Jun 2017 09:18:14 +0200 (CEST) Received: by mail-wr0-f175.google.com with SMTP id v111so26557264wrc.3 for ; Fri, 09 Jun 2017 00:18:14 -0700 (PDT) In-Reply-To: List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, 8 Jun 2017 21:46:00 +0000, "Patil, Harish" wrote: > >Hi Rasesh, > > > >On Wed, 7 Jun 2017 00:43:48 -0700, Rasesh Mody > >wrote: =20 > >> From: Harish Patil > >>=20 > >> 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 packet= s. > >>=20 > >> Signed-off-by: Harish Patil > >> --- > >> lib/librte_mbuf/rte_mbuf.c | 2 ++ > >> lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++------- > >> 2 files changed, 12 insertions(+), 7 deletions(-) > >>=20 > >> 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) =20 > > > >I'm not sure it is suitable to change the value of an existing > >flag, since it breaks the ABI. > > > > =20 > >> /** > >> - * 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) > >> =20 > >> /** > >> * Second VLAN insertion (QinQ) flag. =20 > > > >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.= =20 >=20 > [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=E2=80=99t 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