From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Chilikin, Andrey" <andrey.chilikin@intel.com>
Cc: dev@dpdk.org
Subject: Re: [PATCH] examples: add ip version check for l3fwd app
Date: Wed, 03 Jun 2015 10:27:11 +0200 [thread overview]
Message-ID: <3487943.gKaMDFnUlK@xps13> (raw)
In-Reply-To: <AAC06825A3B29643AF5372F5E0DDF053350EE301@IRSMSX106.ger.corp.intel.com>
2015-06-03 08:24, Chilikin, Andrey:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2015-05-15 17:08, Andrey Chilikin:
> > > Added optional ip version check to l3fwd app to allow to detect the ip
> > > version if mbuf ol_flags are not set in case of running in a VM with
> > > emulated network controllers
> > >
> > > Signed-off-by: Andrey Chilikin <andrey.chilikin@intel.com>
> > [...]
> > > +#define DO_IP_VERSION_CHECK 0
> > [...]
> > > @@ -953,6 +955,15 @@ l3fwd_simple_forward(struct rte_mbuf *m, uint8_t
> > portid, struct lcore_conf *qcon
> > > void *d_addr_bytes;
> > > uint8_t dst_port;
> > >
> > > +#if DO_IP_VERSION_CHECK
> > > + if (!(m->ol_flags & (PKT_RX_IPV4_HDR | PKT_RX_IPV6_HDR))) {
> > > + uint8_t ip_ver = *(uint8_t *)(rte_pktmbuf_mtod(m, unsigned
> > char *) +
> > > + sizeof(struct ether_hdr)) >> 4;
> > > + if (ip_ver == 4)
> > > + m->ol_flags |= PKT_RX_IPV4_HDR;
> > > + }
> > > +#endif
> >
> > You are adding dead code. When ol_flags will be updated, it will be forget until
> > someone enables it.
> > In general, compile-time configurations are avoided and it's even worst when
> > this is hidden and not easily testable like here.
>
> Agree, we already thinking about creating separate example l3fwd-vm to cover emulated network controllers.
Or you can try to implement some callbacks and choose the implementation
at init when detecting the environment.
prev parent reply other threads:[~2015-06-03 8:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-15 16:08 [PATCH] examples: add ip version check for l3fwd app Andrey Chilikin
2015-06-03 8:16 ` Thomas Monjalon
2015-06-03 8:24 ` Chilikin, Andrey
2015-06-03 8:27 ` Thomas Monjalon [this message]
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=3487943.gKaMDFnUlK@xps13 \
--to=thomas.monjalon@6wind.com \
--cc=andrey.chilikin@intel.com \
--cc=dev@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.