[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
