All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dai, Wei" <wei.dai@intel.com>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [PATCH] examples/ip_fragmentation: fix check of packet type
Date: Mon, 13 Mar 2017 03:29:18 +0000	[thread overview]
Message-ID: <49759EB36A64CF4892C1AFEC9231E8D63A377512@PGSMSX106.gar.corp.intel.com> (raw)
In-Reply-To: <2601191342CEEE43887BDE71AB9772583FACD1BD@IRSMSX109.ger.corp.intel.com>

Hi, Konstantin
I see your point.
I think your method can work.
But I think your method is a bit complex. As you method need to add more codes.
Anyway this is a simple bug.
How do you think now ?

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, March 10, 2017 8:21 PM
> To: Dai, Wei <wei.dai@intel.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [PATCH] examples/ip_fragmentation: fix check of packet type
> 
> Hi Wei,
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Friday, March 10, 2017 3:28 AM
> > To: dev@dpdk.org; Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > Cc: Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
> > Subject: [PATCH] examples/ip_fragmentation: fix check of packet type
> >
> > The packet_type in mbuf is not correctly filled by ixgbe 82599 NIC.
> > To use the ether_type in ethernet header to check packet type is more
> > reliaber.
> >
> > Fixes: 3c0184cc0c60 ("examples: replace some offload flags with packet
> > type")
> > Fixes: ab351fe1c95c ("mbuf: remove packet type from offload flags")
> >
> > Cc: stable@dpdk.org
> >
> > Reported-by: Fangfang Wei <fangfangx.wei@intel.com>
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > Tested-by: Fagnfang Wei <fangfangx.wei@intel.com>
> > ---
> >  examples/ip_fragmentation/main.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/examples/ip_fragmentation/main.c
> > b/examples/ip_fragmentation/main.c
> > index e1e32c6..8612984 100644
> > --- a/examples/ip_fragmentation/main.c
> > +++ b/examples/ip_fragmentation/main.c
> > @@ -268,6 +268,7 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct
> lcore_queue_conf *qconf,
> >  	uint32_t i, len, next_hop_ipv4;
> >  	uint8_t next_hop_ipv6, port_out, ipv6;
> >  	int32_t len2;
> > +	struct ether_hdr *eth_hdr;
> >
> >  	ipv6 = 0;
> >  	rxq = &qconf->rx_queue_list[queueid]; @@ -276,13 +277,14 @@
> > l3fwd_simple_forward(struct rte_mbuf *m, struct lcore_queue_conf *qconf,
> >  	port_out = port_in;
> >
> >  	/* Remove the Ethernet header and trailer from the input packet */
> > +	eth_hdr = rte_pktmbuf_mtod(m, struct ether_hdr *);
> >  	rte_pktmbuf_adj(m, (uint16_t)sizeof(struct ether_hdr));
> >
> >  	/* Build transmission burst */
> >  	len = qconf->tx_mbufs[port_out].len;
> >
> >  	/* if this is an IPv4 packet */
> > -	if (RTE_ETH_IS_IPV4_HDR(m->packet_type)) {
> > +	if (eth_hdr->ether_type == rte_cpu_to_be_16(ETHER_TYPE_IPv4)) {
> >  		struct ipv4_hdr *ip_hdr;
> >  		uint32_t ip_dst;
> >  		/* Read the lookup key (i.e. ip_dst) from the input packet */ @@
> > -316,7 +318,7 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct
> lcore_queue_conf *qconf,
> >  			if (unlikely (len2 < 0))
> >  				return;
> >  		}
> > -	} else if (RTE_ETH_IS_IPV6_HDR(m->packet_type)) {
> > +	} else if (eth_hdr->ether_type == rte_be_to_cpu_16(ETHER_TYPE_IPv6))
> > +{
> >  		/* if this is an IPv6 packet */
> >  		struct ipv6_hdr *ip_hdr;
> >
> > @@ -363,8 +365,8 @@ l3fwd_simple_forward(struct rte_mbuf *m, struct
> lcore_queue_conf *qconf,
> >  		void *d_addr_bytes;
> >
> >  		m = qconf->tx_mbufs[port_out].m_table[i];
> > -		struct ether_hdr *eth_hdr = (struct ether_hdr *)
> > -			rte_pktmbuf_prepend(m, (uint16_t)sizeof(struct ether_hdr));
> > +		eth_hdr = (struct ether_hdr *)rte_pktmbuf_prepend(m,
> > +			(uint16_t)sizeof(struct ether_hdr));
> >  		if (eth_hdr == NULL) {
> >  			rte_panic("No headroom in mbuf.\n");
> >  		}
> 
> Thanks for the fix.
> Would it be more convenient to do what l3fwd does:
> Check what ptype capabilities are provided by HW, if no ptype support detected,
> then install an RX callback?
> Konstantin
> 
> > --
> > 2.7.4

  reply	other threads:[~2017-03-13  3:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-10  3:28 [PATCH] examples/ip_fragmentation: fix check of packet type Wei Dai
2017-03-10  3:36 ` Dai, Wei
2017-03-10 12:21 ` Ananyev, Konstantin
2017-03-13  3:29   ` Dai, Wei [this message]
2017-03-13  7:01     ` Ananyev, Konstantin
2017-03-13  7:56       ` Dai, Wei
2017-03-13 22:13         ` Ananyev, Konstantin
2017-03-14  1:18           ` Dai, Wei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49759EB36A64CF4892C1AFEC9231E8D63A377512@PGSMSX106.gar.corp.intel.com \
    --to=wei.dai@intel.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=stable@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.