[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
2015-07-24 15:06, Sanford, Robert: > I just noticed a few minor typos in comments: [...] > >On Thu, Jul 16, 2015 at 01:50:12AM +0200, Thomas Monjalon wrote: > >> A packet is tunnelled if the tunnel type is identified or if it has > >> an inner part. > >> > >> Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > >> > >> Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") > >> > >> Signed-off-by: Thomas Monjalon > > > >Makes sense. > > > >Acked-by: Adrien Mazarguil Applied with typo fixes.
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
I just noticed a few minor typos in comments: diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 917dd59..6352c32 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -680,14 +680,14 @@ extern "C" { /** * Check if the (outer) L3 header is IPv4. To avoid comparing IPv4 types one by * one, bit 4 is selected to be used for IPv4 only. Then checking bit 4 can - * determin if it is an IPV4 packet. + * determine if it is an IPV4 packet. */ #define RTE_ETH_IS_IPV4_HDR(ptype) ((ptype) & RTE_PTYPE_L3_IPV4) /** - * Check if the (outer) L3 header is IPv4. To avoid comparing IPv4 types one by - * one, bit 6 is selected to be used for IPv4 only. Then checking bit 6 can - * determin if it is an IPV4 packet. + * Check if the (outer) L3 header is IPv6. To avoid comparing IPv6 types one by + * one, bit 6 is selected to be used for IPv6 only. Then checking bit 6 can + * determine if it is an IPV6 packet. */ #define RTE_ETH_IS_IPV6_HDR(ptype) ((ptype) & RTE_PTYPE_L3_IPV6) -- Regards, Robert >On Thu, Jul 16, 2015 at 01:50:12AM +0200, Thomas Monjalon wrote: >> A packet is tunnelled if the tunnel type is identified or if it has >> an inner part. >> >> Fix also a typo in RTE_PTYPE_INNER_L3_MASK. >> >> Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") >> >> Signed-off-by: Thomas Monjalon > >Makes sense. > >Acked-by: Adrien Mazarguil > >-- >Adrien Mazarguil >6WIND
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
On Thu, Jul 16, 2015 at 01:50:12AM +0200, Thomas Monjalon wrote: > A packet is tunnelled if the tunnel type is identified or if it has > an inner part. > > Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > > Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") > > Signed-off-by: Thomas Monjalon Makes sense. Acked-by: Adrien Mazarguil -- Adrien Mazarguil 6WIND
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
2015-07-16 00:36, Zhang, Helin: > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > 2015-07-15 23:57, Zhang, Helin: > > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > > A packet is tunnelled if the tunnel type is identified or if it has an > > > > inner part. > > > > > > > > Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > > > > > > > > Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet > > > > types") > > > > > > > > Signed-off-by: Thomas Monjalon > > |...] > > > > /* Check if it is a tunneling packet */ -#define > > > > RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & RTE_PTYPE_TUNNEL_MASK) > > > > +#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & > > > > +(RTE_PTYPE_TUNNEL_MASK | \ RTE_PTYPE_INNER_L2_MASK | \ > > > > +RTE_PTYPE_INNER_L3_MASK | \ > > > > + RTE_PTYPE_INNER_L4_MASK)) > > > > > > Could you help to explain more of why here? > > > My understanding is that if an inner one can be recognized, there must be > > > a > > tunnel type there. > > > > Not always. > > It was my comment in mlx4 patch: > > http://dpdk.org/ml/archives/dev/2015-July/021702.html > > Currently we can know that mlx4 has detected a tunnel but don't know which > > one. > > I'd suggest to do more in mlx4 driver, rather than adding more checks in this > macro. > If it can detect the inner l2/l3 type, the driver should tell it is a > tunneled packet. > If it cannot know which tunnel it is, I'd suggest to add one more tunnel type > of > RTE_PTYPE_TUNNEL_UNKNOWN. > Two reasons: > - PMD should tell enough info to high level caller or application. It should > be clear enough. RTE_PTYPE_TUNNEL_UNKNOWN doesn't make it clearer, IMHO. > - Adding more checks in those macro results in more cpu cycles for other NICs > to check > the packet types. Not sure. It only extends the mask to check. It shouldn't add more cpu cycle.
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
2015-07-15 23:57, Zhang, Helin: > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > A packet is tunnelled if the tunnel type is identified or if it has an > > inner part. > > > > Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > > > > Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") > > > > Signed-off-by: Thomas Monjalon |...] > > /* Check if it is a tunneling packet */ > > -#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & RTE_PTYPE_TUNNEL_MASK) > > +#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & (RTE_PTYPE_TUNNEL_MASK | \ > > + RTE_PTYPE_INNER_L2_MASK | \ > > + RTE_PTYPE_INNER_L3_MASK | \ > > + RTE_PTYPE_INNER_L4_MASK)) > > Could you help to explain more of why here? > My understanding is that if an inner one can be recognized, there must be a > tunnel type there. Not always. It was my comment in mlx4 patch: http://dpdk.org/ml/archives/dev/2015-July/021702.html Currently we can know that mlx4 has detected a tunnel but don't know which one.
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
A packet is tunnelled if the tunnel type is identified or if it has an inner part. Fix also a typo in RTE_PTYPE_INNER_L3_MASK. Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") Signed-off-by: Thomas Monjalon --- app/test-pmd/rxonly.c | 2 +- lib/librte_mbuf/rte_mbuf.h | 7 +-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c index 41871d3..ee7fd8d 100644 --- a/app/test-pmd/rxonly.c +++ b/app/test-pmd/rxonly.c @@ -288,7 +288,7 @@ pkt_burst_receive(struct fwd_stream *fs) } /* inner L3 packet type */ - ptype = mb->packet_type & RTE_PTYPE_INNER_INNER_L3_MASK; + ptype = mb->packet_type & RTE_PTYPE_INNER_L3_MASK; switch (ptype) { case RTE_PTYPE_INNER_L3_IPV4: printf(" - Inner L3 type: IPV4"); diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 115c560..dbd9095 100644 --- a/lib/librte_mbuf/rte_mbuf.h +++ b/lib/librte_mbuf/rte_mbuf.h @@ -595,7 +595,7 @@ extern "C" { /** * Mask of inner layer 3 packet types. */ -#define RTE_PTYPE_INNER_INNER_L3_MASK 0x00f0 +#define RTE_PTYPE_INNER_L3_MASK 0x00f0 /** * TCP (Transmission Control Protocol) packet type. * It is used for inner packet only. @@ -689,7 +689,10 @@ extern "C" { #define RTE_ETH_IS_IPV6_HDR(ptype) ((ptype) & RTE_PTYPE_L3_IPV6) /* Check if it is a tunneling packet */ -#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & RTE_PTYPE_TUNNEL_MASK) +#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & (RTE_PTYPE_TUNNEL_MASK | \ + RTE_PTYPE_INNER_L2_MASK | \ + RTE_PTYPE_INNER_L3_MASK | \ + RTE_PTYPE_INNER_L4_MASK)) #endif /* RTE_NEXT_ABI */ /** -- 2.4.2
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, July 15, 2015 5:13 PM > To: Zhang, Helin > Cc: olivier.matz at 6wind.com; dev at dpdk.org > Subject: Re: [PATCH] mbuf: fix tunnel flags check > > 2015-07-15 23:57, Zhang, Helin: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > A packet is tunnelled if the tunnel type is identified or if it has an > > > inner part. > > > > > > Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > > > > > > Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet > > > types") > > > > > > Signed-off-by: Thomas Monjalon > |...] > > > /* Check if it is a tunneling packet */ -#define > > > RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & RTE_PTYPE_TUNNEL_MASK) > > > +#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & > > > +(RTE_PTYPE_TUNNEL_MASK | \ RTE_PTYPE_INNER_L2_MASK | \ > > > +RTE_PTYPE_INNER_L3_MASK | \ > > > + RTE_PTYPE_INNER_L4_MASK)) > > > > Could you help to explain more of why here? > > My understanding is that if an inner one can be recognized, there must be a > tunnel type there. > > Not always. > It was my comment in mlx4 patch: > http://dpdk.org/ml/archives/dev/2015-July/021702.html > Currently we can know that mlx4 has detected a tunnel but don't know which > one. I'd suggest to do more in mlx4 driver, rather than adding more checks in this macro. If it can detect the inner l2/l3 type, the driver should tell it is a tunneled packet. If it cannot know which tunnel it is, I'd suggest to add one more tunnel type of RTE_PTYPE_TUNNEL_UNKNOWN. Two reasons: - PMD should tell enough info to high level caller or application. It should be clear enough. - Adding more checks in those macro results in more cpu cycles for other NICs to check the packet types. Regards, Helin
[dpdk-dev] [PATCH] mbuf: fix tunnel flags check
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, July 15, 2015 4:50 PM > To: Zhang, Helin > Cc: olivier.matz at 6wind.com; dev at dpdk.org > Subject: [PATCH] mbuf: fix tunnel flags check > > A packet is tunnelled if the tunnel type is identified or if it has an inner > part. > > Fix also a typo in RTE_PTYPE_INNER_L3_MASK. > > Fixes: f295a00a2b44 ("mbuf: add definitions of unified packet types") > > Signed-off-by: Thomas Monjalon > --- > app/test-pmd/rxonly.c | 2 +- > lib/librte_mbuf/rte_mbuf.h | 7 +-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c index > 41871d3..ee7fd8d 100644 > --- a/app/test-pmd/rxonly.c > +++ b/app/test-pmd/rxonly.c > @@ -288,7 +288,7 @@ pkt_burst_receive(struct fwd_stream *fs) > } > > /* inner L3 packet type */ > - ptype = mb->packet_type & > RTE_PTYPE_INNER_INNER_L3_MASK; > + ptype = mb->packet_type & RTE_PTYPE_INNER_L3_MASK; > switch (ptype) { > case RTE_PTYPE_INNER_L3_IPV4: > printf(" - Inner L3 type: IPV4"); > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index > 115c560..dbd9095 100644 > --- a/lib/librte_mbuf/rte_mbuf.h > +++ b/lib/librte_mbuf/rte_mbuf.h > @@ -595,7 +595,7 @@ extern "C" { > /** > * Mask of inner layer 3 packet types. > */ > -#define RTE_PTYPE_INNER_INNER_L3_MASK 0x00f0 > +#define RTE_PTYPE_INNER_L3_MASK 0x00f0 > /** > * TCP (Transmission Control Protocol) packet type. > * It is used for inner packet only. > @@ -689,7 +689,10 @@ extern "C" { > #define RTE_ETH_IS_IPV6_HDR(ptype) ((ptype) & RTE_PTYPE_L3_IPV6) > > /* Check if it is a tunneling packet */ -#define RTE_ETH_IS_TUNNEL_PKT(ptype) > ((ptype) & RTE_PTYPE_TUNNEL_MASK) > +#define RTE_ETH_IS_TUNNEL_PKT(ptype) ((ptype) & > (RTE_PTYPE_TUNNEL_MASK | \ > + > RTE_PTYPE_INNER_L2_MASK | \ > + > RTE_PTYPE_INNER_L3_MASK | \ > + > +RTE_PTYPE_INNER_L4_MASK)) > #endif /* RTE_NEXT_ABI */ Could you help to explain more of why here? My understanding is that if an inner one can be recognized, there must be a tunnel type there. Regards, Helin > > /** > -- > 2.4.2