[ovs-dev] dp-packet.h: move funcs to be within cond block

Message ID 20180925210804.12226-1-fbl@sysclose.org
State Accepted
Delegated to: Ian Stokes
Headers show
Series
  • [ovs-dev] dp-packet.h: move funcs to be within cond block
Related show

Commit Message

Flavio Leitner Sept. 25, 2018, 9:08 p.m.
There is already an ifdef DPDK_NETDEV block, so instead of checking
on each and every function, move them to the right block.

No functional change.

Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
 lib/dp-packet.h | 260 +++++++++++++++++++++++++++++++-------------------------
 1 file changed, 146 insertions(+), 114 deletions(-)

Comments

Lam, Tiago Oct. 2, 2018, 9:32 a.m. | #1
(Sending this again as yesterday's email seems to have been dropped.)

Hi Flavio,

Thanks for the patch. Looks good to me, it compiles with both DPDK and
without DPDK linked, and the check-system-userspace tests pass.

Acked-by: Tiago Lam <tiago.lam@intel.com>

On 25/09/2018 22:08, Flavio Leitner wrote:
> There is already an ifdef DPDK_NETDEV block, so instead of checking
> on each and every function, move them to the right block.
> 
> No functional change.
> 
> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  lib/dp-packet.h | 260 +++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 146 insertions(+), 114 deletions(-)
> 
> diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> index ba91e5891..5b4c9c7a3 100644
> --- a/lib/dp-packet.h
> +++ b/lib/dp-packet.h
> @@ -465,113 +465,142 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
>  {
>      b->mbuf.buf_len = s;
>  }
> -#else
> -static inline void *
> -dp_packet_base(const struct dp_packet *b)
> +
> +/* Returns the RSS hash of the packet 'p'.  Note that the returned value is
> + * correct only if 'dp_packet_rss_valid(p)' returns true */
> +static inline uint32_t
> +dp_packet_get_rss_hash(struct dp_packet *p)
>  {
> -    return b->base_;
> +    return p->mbuf.hash.rss;
>  }
>  
>  static inline void
> -dp_packet_set_base(struct dp_packet *b, void *d)
> +dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
>  {
> -    b->base_ = d;
> +    p->mbuf.hash.rss = hash;
> +    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
>  }
>  
> -static inline uint32_t
> -dp_packet_size(const struct dp_packet *b)
> +static inline bool
> +dp_packet_rss_valid(struct dp_packet *p)
>  {
> -    return b->size_;
> +    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
>  }
>  
>  static inline void
> -dp_packet_set_size(struct dp_packet *b, uint32_t v)
> +dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
>  {
> -    b->size_ = v;
>  }
>  
> -static inline uint16_t
> -__packet_data(const struct dp_packet *b)
> +static inline void
> +dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
>  {
> -    return b->data_ofs;
> +    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
>  }
>  
> +/* This initialization is needed for packets that do not come
> + * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> + * The DPDK rte library will still otherwise manage the mbuf.
> + * We only need to initialize the mbuf ol_flags. */
>  static inline void
> -__packet_set_data(struct dp_packet *b, uint16_t v)
> +dp_packet_mbuf_init(struct dp_packet *p)
>  {
> -    b->data_ofs = v;
> +    p->mbuf.ol_flags = 0;
>  }
>  
> -static inline uint16_t
> -dp_packet_get_allocated(const struct dp_packet *b)
> +static inline bool
> +dp_packet_ip_checksum_valid(struct dp_packet *p)
>  {
> -    return b->allocated_;
> +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> +            PKT_RX_IP_CKSUM_GOOD;
>  }
>  
> -static inline void
> -dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> +static inline bool
> +dp_packet_ip_checksum_bad(struct dp_packet *p)
>  {
> -    b->allocated_ = s;
> +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> +            PKT_RX_IP_CKSUM_BAD;
> +}
> +
> +static inline bool
> +dp_packet_l4_checksum_valid(struct dp_packet *p)
> +{
> +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> +            PKT_RX_L4_CKSUM_GOOD;
> +}
> +
> +static inline bool
> +dp_packet_l4_checksum_bad(struct dp_packet *p)
> +{
> +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> +            PKT_RX_L4_CKSUM_BAD;
>  }
> -#endif
>  
>  static inline void
> -dp_packet_reset_cutlen(struct dp_packet *b)
> +reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
>  {
> -    b->cutlen = 0;
> +    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
> +                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
>  }
>  
> -static inline uint32_t
> -dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> +static inline bool
> +dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
>  {
> -    if (max_len < ETH_HEADER_LEN) {
> -        max_len = ETH_HEADER_LEN;
> +    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> +        *mark = p->mbuf.hash.fdir.hi;
> +        return true;
>      }
>  
> -    if (max_len >= dp_packet_size(b)) {
> -        b->cutlen = 0;
> -    } else {
> -        b->cutlen = dp_packet_size(b) - max_len;
> -    }
> -    return b->cutlen;
> +    return false;
>  }
>  
> -static inline uint32_t
> -dp_packet_get_cutlen(const struct dp_packet *b)
> +#else /* DPDK_NETDEV */
> +static inline void *
> +dp_packet_base(const struct dp_packet *b)
>  {
> -    /* Always in valid range if user uses dp_packet_set_cutlen. */
> -    return b->cutlen;
> +    return b->base_;
> +}
> +
> +static inline void
> +dp_packet_set_base(struct dp_packet *b, void *d)
> +{
> +    b->base_ = d;
>  }
>  
>  static inline uint32_t
> -dp_packet_get_send_len(const struct dp_packet *b)
> +dp_packet_size(const struct dp_packet *b)
>  {
> -    return dp_packet_size(b) - dp_packet_get_cutlen(b);
> +    return b->size_;
>  }
>  
> -static inline void *
> -dp_packet_data(const struct dp_packet *b)
> +static inline void
> +dp_packet_set_size(struct dp_packet *b, uint32_t v)
>  {
> -    return __packet_data(b) != UINT16_MAX
> -           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> +    b->size_ = v;
> +}
> +
> +static inline uint16_t
> +__packet_data(const struct dp_packet *b)
> +{
> +    return b->data_ofs;
>  }
>  
>  static inline void
> -dp_packet_set_data(struct dp_packet *b, void *data)
> +__packet_set_data(struct dp_packet *b, uint16_t v)
>  {
> -    if (data) {
> -        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
> -    } else {
> -        __packet_set_data(b, UINT16_MAX);
> -    }
> +    b->data_ofs = v;
> +}
> +
> +static inline uint16_t
> +dp_packet_get_allocated(const struct dp_packet *b)
> +{
> +    return b->allocated_;
>  }
>  
>  static inline void
> -dp_packet_reset_packet(struct dp_packet *b, int off)
> +dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
>  {
> -    dp_packet_set_size(b, dp_packet_size(b) - off);
> -    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
> -    dp_packet_reset_offsets(b);
> +    b->allocated_ = s;
>  }
>  
>  /* Returns the RSS hash of the packet 'p'.  Note that the returned value is
> @@ -579,130 +608,133 @@ dp_packet_reset_packet(struct dp_packet *b, int off)
>  static inline uint32_t
>  dp_packet_get_rss_hash(struct dp_packet *p)
>  {
> -#ifdef DPDK_NETDEV
> -    return p->mbuf.hash.rss;
> -#else
>      return p->rss_hash;
> -#endif
>  }
>  
>  static inline void
>  dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
>  {
> -#ifdef DPDK_NETDEV
> -    p->mbuf.hash.rss = hash;
> -    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> -#else
>      p->rss_hash = hash;
>      p->rss_hash_valid = true;
> -#endif
>  }
>  
>  static inline bool
>  dp_packet_rss_valid(struct dp_packet *p)
>  {
> -#ifdef DPDK_NETDEV
> -    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> -#else
>      return p->rss_hash_valid;
> -#endif
>  }
>  
>  static inline void
> -dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> +dp_packet_rss_invalidate(struct dp_packet *p)
>  {
> -#ifndef DPDK_NETDEV
>      p->rss_hash_valid = false;
> -#endif
>  }
>  
>  static inline void
>  dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> -#endif
>  }
>  
> -/* This initialization is needed for packets that do not come
> - * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> - * The DPDK rte library will still otherwise manage the mbuf.
> - * We only need to initialize the mbuf ol_flags. */
>  static inline void
>  dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    p->mbuf.ol_flags = 0;
> -#endif
>  }
>  
>  static inline bool
>  dp_packet_ip_checksum_valid(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> -            PKT_RX_IP_CKSUM_GOOD;
> -#else
>      return false;
> -#endif
>  }
>  
>  static inline bool
>  dp_packet_ip_checksum_bad(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> -            PKT_RX_IP_CKSUM_BAD;
> -#else
>      return false;
> -#endif
>  }
>  
>  static inline bool
>  dp_packet_l4_checksum_valid(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> -            PKT_RX_L4_CKSUM_GOOD;
> -#else
>      return false;
> -#endif
>  }
>  
>  static inline bool
>  dp_packet_l4_checksum_bad(struct dp_packet *p OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> -            PKT_RX_L4_CKSUM_BAD;
> -#else
>      return false;
> -#endif
>  }
>  
> -#ifdef DPDK_NETDEV
>  static inline void
> -reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> +reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED)
>  {
> -    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
> -                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
>  }
> -#else
> -#define reset_dp_packet_checksum_ol_flags(arg)
> -#endif
>  
>  static inline bool
>  dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
>                          uint32_t *mark OVS_UNUSED)
>  {
> -#ifdef DPDK_NETDEV
> -    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> -        *mark = p->mbuf.hash.fdir.hi;
> -        return true;
> -    }
> -#endif
>      return false;
>  }
> +#endif /* DPDK_NETDEV */
> +
> +static inline void
> +dp_packet_reset_cutlen(struct dp_packet *b)
> +{
> +    b->cutlen = 0;
> +}
> +
> +static inline uint32_t
> +dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> +{
> +    if (max_len < ETH_HEADER_LEN) {
> +        max_len = ETH_HEADER_LEN;
> +    }
> +
> +    if (max_len >= dp_packet_size(b)) {
> +        b->cutlen = 0;
> +    } else {
> +        b->cutlen = dp_packet_size(b) - max_len;
> +    }
> +    return b->cutlen;
> +}
> +
> +static inline uint32_t
> +dp_packet_get_cutlen(const struct dp_packet *b)
> +{
> +    /* Always in valid range if user uses dp_packet_set_cutlen. */
> +    return b->cutlen;
> +}
> +
> +static inline uint32_t
> +dp_packet_get_send_len(const struct dp_packet *b)
> +{
> +    return dp_packet_size(b) - dp_packet_get_cutlen(b);
> +}
> +
> +static inline void *
> +dp_packet_data(const struct dp_packet *b)
> +{
> +    return __packet_data(b) != UINT16_MAX
> +           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> +}
> +
> +static inline void
> +dp_packet_set_data(struct dp_packet *b, void *data)
> +{
> +    if (data) {
> +        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
> +    } else {
> +        __packet_set_data(b, UINT16_MAX);
> +    }
> +}
> +
> +static inline void
> +dp_packet_reset_packet(struct dp_packet *b, int off)
> +{
> +    dp_packet_set_size(b, dp_packet_size(b) - off);
> +    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
> +    dp_packet_reset_offsets(b);
> +}
>  
>  enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */
>  
>
Ben Pfaff Oct. 2, 2018, 5:53 p.m. | #2
For what it's worth, Ian, I'm hoping that you'll take a look at this and
decide whether to merge it.

On Tue, Oct 02, 2018 at 10:32:40AM +0100, Lam, Tiago wrote:
> (Sending this again as yesterday's email seems to have been dropped.)
> 
> Hi Flavio,
> 
> Thanks for the patch. Looks good to me, it compiles with both DPDK and
> without DPDK linked, and the check-system-userspace tests pass.
> 
> Acked-by: Tiago Lam <tiago.lam@intel.com>
> 
> On 25/09/2018 22:08, Flavio Leitner wrote:
> > There is already an ifdef DPDK_NETDEV block, so instead of checking
> > on each and every function, move them to the right block.
> > 
> > No functional change.
> > 
> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> >  lib/dp-packet.h | 260 +++++++++++++++++++++++++++++++-------------------------
> >  1 file changed, 146 insertions(+), 114 deletions(-)
> > 
> > diff --git a/lib/dp-packet.h b/lib/dp-packet.h
> > index ba91e5891..5b4c9c7a3 100644
> > --- a/lib/dp-packet.h
> > +++ b/lib/dp-packet.h
> > @@ -465,113 +465,142 @@ dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> >  {
> >      b->mbuf.buf_len = s;
> >  }
> > -#else
> > -static inline void *
> > -dp_packet_base(const struct dp_packet *b)
> > +
> > +/* Returns the RSS hash of the packet 'p'.  Note that the returned value is
> > + * correct only if 'dp_packet_rss_valid(p)' returns true */
> > +static inline uint32_t
> > +dp_packet_get_rss_hash(struct dp_packet *p)
> >  {
> > -    return b->base_;
> > +    return p->mbuf.hash.rss;
> >  }
> >  
> >  static inline void
> > -dp_packet_set_base(struct dp_packet *b, void *d)
> > +dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
> >  {
> > -    b->base_ = d;
> > +    p->mbuf.hash.rss = hash;
> > +    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> >  }
> >  
> > -static inline uint32_t
> > -dp_packet_size(const struct dp_packet *b)
> > +static inline bool
> > +dp_packet_rss_valid(struct dp_packet *p)
> >  {
> > -    return b->size_;
> > +    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> >  }
> >  
> >  static inline void
> > -dp_packet_set_size(struct dp_packet *b, uint32_t v)
> > +dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> >  {
> > -    b->size_ = v;
> >  }
> >  
> > -static inline uint16_t
> > -__packet_data(const struct dp_packet *b)
> > +static inline void
> > +dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
> >  {
> > -    return b->data_ofs;
> > +    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> >  }
> >  
> > +/* This initialization is needed for packets that do not come
> > + * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> > + * The DPDK rte library will still otherwise manage the mbuf.
> > + * We only need to initialize the mbuf ol_flags. */
> >  static inline void
> > -__packet_set_data(struct dp_packet *b, uint16_t v)
> > +dp_packet_mbuf_init(struct dp_packet *p)
> >  {
> > -    b->data_ofs = v;
> > +    p->mbuf.ol_flags = 0;
> >  }
> >  
> > -static inline uint16_t
> > -dp_packet_get_allocated(const struct dp_packet *b)
> > +static inline bool
> > +dp_packet_ip_checksum_valid(struct dp_packet *p)
> >  {
> > -    return b->allocated_;
> > +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > +            PKT_RX_IP_CKSUM_GOOD;
> >  }
> >  
> > -static inline void
> > -dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> > +static inline bool
> > +dp_packet_ip_checksum_bad(struct dp_packet *p)
> >  {
> > -    b->allocated_ = s;
> > +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > +            PKT_RX_IP_CKSUM_BAD;
> > +}
> > +
> > +static inline bool
> > +dp_packet_l4_checksum_valid(struct dp_packet *p)
> > +{
> > +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > +            PKT_RX_L4_CKSUM_GOOD;
> > +}
> > +
> > +static inline bool
> > +dp_packet_l4_checksum_bad(struct dp_packet *p)
> > +{
> > +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > +            PKT_RX_L4_CKSUM_BAD;
> >  }
> > -#endif
> >  
> >  static inline void
> > -dp_packet_reset_cutlen(struct dp_packet *b)
> > +reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> >  {
> > -    b->cutlen = 0;
> > +    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
> > +                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
> >  }
> >  
> > -static inline uint32_t
> > -dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> > +static inline bool
> > +dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
> >  {
> > -    if (max_len < ETH_HEADER_LEN) {
> > -        max_len = ETH_HEADER_LEN;
> > +    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> > +        *mark = p->mbuf.hash.fdir.hi;
> > +        return true;
> >      }
> >  
> > -    if (max_len >= dp_packet_size(b)) {
> > -        b->cutlen = 0;
> > -    } else {
> > -        b->cutlen = dp_packet_size(b) - max_len;
> > -    }
> > -    return b->cutlen;
> > +    return false;
> >  }
> >  
> > -static inline uint32_t
> > -dp_packet_get_cutlen(const struct dp_packet *b)
> > +#else /* DPDK_NETDEV */
> > +static inline void *
> > +dp_packet_base(const struct dp_packet *b)
> >  {
> > -    /* Always in valid range if user uses dp_packet_set_cutlen. */
> > -    return b->cutlen;
> > +    return b->base_;
> > +}
> > +
> > +static inline void
> > +dp_packet_set_base(struct dp_packet *b, void *d)
> > +{
> > +    b->base_ = d;
> >  }
> >  
> >  static inline uint32_t
> > -dp_packet_get_send_len(const struct dp_packet *b)
> > +dp_packet_size(const struct dp_packet *b)
> >  {
> > -    return dp_packet_size(b) - dp_packet_get_cutlen(b);
> > +    return b->size_;
> >  }
> >  
> > -static inline void *
> > -dp_packet_data(const struct dp_packet *b)
> > +static inline void
> > +dp_packet_set_size(struct dp_packet *b, uint32_t v)
> >  {
> > -    return __packet_data(b) != UINT16_MAX
> > -           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> > +    b->size_ = v;
> > +}
> > +
> > +static inline uint16_t
> > +__packet_data(const struct dp_packet *b)
> > +{
> > +    return b->data_ofs;
> >  }
> >  
> >  static inline void
> > -dp_packet_set_data(struct dp_packet *b, void *data)
> > +__packet_set_data(struct dp_packet *b, uint16_t v)
> >  {
> > -    if (data) {
> > -        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
> > -    } else {
> > -        __packet_set_data(b, UINT16_MAX);
> > -    }
> > +    b->data_ofs = v;
> > +}
> > +
> > +static inline uint16_t
> > +dp_packet_get_allocated(const struct dp_packet *b)
> > +{
> > +    return b->allocated_;
> >  }
> >  
> >  static inline void
> > -dp_packet_reset_packet(struct dp_packet *b, int off)
> > +dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> >  {
> > -    dp_packet_set_size(b, dp_packet_size(b) - off);
> > -    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
> > -    dp_packet_reset_offsets(b);
> > +    b->allocated_ = s;
> >  }
> >  
> >  /* Returns the RSS hash of the packet 'p'.  Note that the returned value is
> > @@ -579,130 +608,133 @@ dp_packet_reset_packet(struct dp_packet *b, int off)
> >  static inline uint32_t
> >  dp_packet_get_rss_hash(struct dp_packet *p)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return p->mbuf.hash.rss;
> > -#else
> >      return p->rss_hash;
> > -#endif
> >  }
> >  
> >  static inline void
> >  dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    p->mbuf.hash.rss = hash;
> > -    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> > -#else
> >      p->rss_hash = hash;
> >      p->rss_hash_valid = true;
> > -#endif
> >  }
> >  
> >  static inline bool
> >  dp_packet_rss_valid(struct dp_packet *p)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> > -#else
> >      return p->rss_hash_valid;
> > -#endif
> >  }
> >  
> >  static inline void
> > -dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> > +dp_packet_rss_invalidate(struct dp_packet *p)
> >  {
> > -#ifndef DPDK_NETDEV
> >      p->rss_hash_valid = false;
> > -#endif
> >  }
> >  
> >  static inline void
> >  dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> > -#endif
> >  }
> >  
> > -/* This initialization is needed for packets that do not come
> > - * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> > - * The DPDK rte library will still otherwise manage the mbuf.
> > - * We only need to initialize the mbuf ol_flags. */
> >  static inline void
> >  dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    p->mbuf.ol_flags = 0;
> > -#endif
> >  }
> >  
> >  static inline bool
> >  dp_packet_ip_checksum_valid(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > -            PKT_RX_IP_CKSUM_GOOD;
> > -#else
> >      return false;
> > -#endif
> >  }
> >  
> >  static inline bool
> >  dp_packet_ip_checksum_bad(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > -            PKT_RX_IP_CKSUM_BAD;
> > -#else
> >      return false;
> > -#endif
> >  }
> >  
> >  static inline bool
> >  dp_packet_l4_checksum_valid(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > -            PKT_RX_L4_CKSUM_GOOD;
> > -#else
> >      return false;
> > -#endif
> >  }
> >  
> >  static inline bool
> >  dp_packet_l4_checksum_bad(struct dp_packet *p OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > -            PKT_RX_L4_CKSUM_BAD;
> > -#else
> >      return false;
> > -#endif
> >  }
> >  
> > -#ifdef DPDK_NETDEV
> >  static inline void
> > -reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> > +reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED)
> >  {
> > -    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
> > -                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
> >  }
> > -#else
> > -#define reset_dp_packet_checksum_ol_flags(arg)
> > -#endif
> >  
> >  static inline bool
> >  dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
> >                          uint32_t *mark OVS_UNUSED)
> >  {
> > -#ifdef DPDK_NETDEV
> > -    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> > -        *mark = p->mbuf.hash.fdir.hi;
> > -        return true;
> > -    }
> > -#endif
> >      return false;
> >  }
> > +#endif /* DPDK_NETDEV */
> > +
> > +static inline void
> > +dp_packet_reset_cutlen(struct dp_packet *b)
> > +{
> > +    b->cutlen = 0;
> > +}
> > +
> > +static inline uint32_t
> > +dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> > +{
> > +    if (max_len < ETH_HEADER_LEN) {
> > +        max_len = ETH_HEADER_LEN;
> > +    }
> > +
> > +    if (max_len >= dp_packet_size(b)) {
> > +        b->cutlen = 0;
> > +    } else {
> > +        b->cutlen = dp_packet_size(b) - max_len;
> > +    }
> > +    return b->cutlen;
> > +}
> > +
> > +static inline uint32_t
> > +dp_packet_get_cutlen(const struct dp_packet *b)
> > +{
> > +    /* Always in valid range if user uses dp_packet_set_cutlen. */
> > +    return b->cutlen;
> > +}
> > +
> > +static inline uint32_t
> > +dp_packet_get_send_len(const struct dp_packet *b)
> > +{
> > +    return dp_packet_size(b) - dp_packet_get_cutlen(b);
> > +}
> > +
> > +static inline void *
> > +dp_packet_data(const struct dp_packet *b)
> > +{
> > +    return __packet_data(b) != UINT16_MAX
> > +           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> > +}
> > +
> > +static inline void
> > +dp_packet_set_data(struct dp_packet *b, void *data)
> > +{
> > +    if (data) {
> > +        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
> > +    } else {
> > +        __packet_set_data(b, UINT16_MAX);
> > +    }
> > +}
> > +
> > +static inline void
> > +dp_packet_reset_packet(struct dp_packet *b, int off)
> > +{
> > +    dp_packet_set_size(b, dp_packet_size(b) - off);
> > +    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
> > +    dp_packet_reset_offsets(b);
> > +}
> >  
> >  enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */
> >  
> > 
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Stokes, Ian Oct. 8, 2018, 5:39 p.m. | #3
> For what it's worth, Ian, I'm hoping that you'll take a look at this and
> decide whether to merge it.

Sure, thanks for flagging Ben, apologies for the delay, I've been on vacation the past 2 weeks but will have time to look at this now.

Thanks
Ian
> 
> On Tue, Oct 02, 2018 at 10:32:40AM +0100, Lam, Tiago wrote:
> > (Sending this again as yesterday's email seems to have been dropped.)
> >
> > Hi Flavio,
> >
> > Thanks for the patch. Looks good to me, it compiles with both DPDK and
> > without DPDK linked, and the check-system-userspace tests pass.
> >
> > Acked-by: Tiago Lam <tiago.lam@intel.com>
> >
> > On 25/09/2018 22:08, Flavio Leitner wrote:
> > > There is already an ifdef DPDK_NETDEV block, so instead of checking
> > > on each and every function, move them to the right block.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > > ---
> > >  lib/dp-packet.h | 260
> > > +++++++++++++++++++++++++++++++-------------------------
> > >  1 file changed, 146 insertions(+), 114 deletions(-)
> > >
> > > diff --git a/lib/dp-packet.h b/lib/dp-packet.h index
> > > ba91e5891..5b4c9c7a3 100644
> > > --- a/lib/dp-packet.h
> > > +++ b/lib/dp-packet.h
> > > @@ -465,113 +465,142 @@ dp_packet_set_allocated(struct dp_packet *b,
> > > uint16_t s)  {
> > >      b->mbuf.buf_len = s;
> > >  }
> > > -#else
> > > -static inline void *
> > > -dp_packet_base(const struct dp_packet *b)
> > > +
> > > +/* Returns the RSS hash of the packet 'p'.  Note that the returned
> > > +value is
> > > + * correct only if 'dp_packet_rss_valid(p)' returns true */ static
> > > +inline uint32_t dp_packet_get_rss_hash(struct dp_packet *p)
> > >  {
> > > -    return b->base_;
> > > +    return p->mbuf.hash.rss;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_set_base(struct dp_packet *b, void *d)
> > > +dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
> > >  {
> > > -    b->base_ = d;
> > > +    p->mbuf.hash.rss = hash;
> > > +    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> > >  }
> > >
> > > -static inline uint32_t
> > > -dp_packet_size(const struct dp_packet *b)
> > > +static inline bool
> > > +dp_packet_rss_valid(struct dp_packet *p)
> > >  {
> > > -    return b->size_;
> > > +    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_set_size(struct dp_packet *b, uint32_t v)
> > > +dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> > >  {
> > > -    b->size_ = v;
> > >  }
> > >
> > > -static inline uint16_t
> > > -__packet_data(const struct dp_packet *b)
> > > +static inline void
> > > +dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
> > >  {
> > > -    return b->data_ofs;
> > > +    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> > >  }
> > >
> > > +/* This initialization is needed for packets that do not come
> > > + * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> > > + * The DPDK rte library will still otherwise manage the mbuf.
> > > + * We only need to initialize the mbuf ol_flags. */
> > >  static inline void
> > > -__packet_set_data(struct dp_packet *b, uint16_t v)
> > > +dp_packet_mbuf_init(struct dp_packet *p)
> > >  {
> > > -    b->data_ofs = v;
> > > +    p->mbuf.ol_flags = 0;
> > >  }
> > >
> > > -static inline uint16_t
> > > -dp_packet_get_allocated(const struct dp_packet *b)
> > > +static inline bool
> > > +dp_packet_ip_checksum_valid(struct dp_packet *p)
> > >  {
> > > -    return b->allocated_;
> > > +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > +            PKT_RX_IP_CKSUM_GOOD;
> > >  }
> > >
> > > -static inline void
> > > -dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> > > +static inline bool
> > > +dp_packet_ip_checksum_bad(struct dp_packet *p)
> > >  {
> > > -    b->allocated_ = s;
> > > +    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > +            PKT_RX_IP_CKSUM_BAD;
> > > +}
> > > +
> > > +static inline bool
> > > +dp_packet_l4_checksum_valid(struct dp_packet *p) {
> > > +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > +            PKT_RX_L4_CKSUM_GOOD;
> > > +}
> > > +
> > > +static inline bool
> > > +dp_packet_l4_checksum_bad(struct dp_packet *p) {
> > > +    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > +            PKT_RX_L4_CKSUM_BAD;
> > >  }
> > > -#endif
> > >
> > >  static inline void
> > > -dp_packet_reset_cutlen(struct dp_packet *b)
> > > +reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> > >  {
> > > -    b->cutlen = 0;
> > > +    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD
> |
> > > +                          PKT_RX_IP_CKSUM_GOOD |
> > > + PKT_RX_IP_CKSUM_BAD);
> > >  }
> > >
> > > -static inline uint32_t
> > > -dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
> > > +static inline bool
> > > +dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
> > >  {
> > > -    if (max_len < ETH_HEADER_LEN) {
> > > -        max_len = ETH_HEADER_LEN;
> > > +    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> > > +        *mark = p->mbuf.hash.fdir.hi;
> > > +        return true;
> > >      }
> > >
> > > -    if (max_len >= dp_packet_size(b)) {
> > > -        b->cutlen = 0;
> > > -    } else {
> > > -        b->cutlen = dp_packet_size(b) - max_len;
> > > -    }
> > > -    return b->cutlen;
> > > +    return false;
> > >  }
> > >
> > > -static inline uint32_t
> > > -dp_packet_get_cutlen(const struct dp_packet *b)
> > > +#else /* DPDK_NETDEV */
> > > +static inline void *
> > > +dp_packet_base(const struct dp_packet *b)
> > >  {
> > > -    /* Always in valid range if user uses dp_packet_set_cutlen. */
> > > -    return b->cutlen;
> > > +    return b->base_;
> > > +}
> > > +
> > > +static inline void
> > > +dp_packet_set_base(struct dp_packet *b, void *d) {
> > > +    b->base_ = d;
> > >  }
> > >
> > >  static inline uint32_t
> > > -dp_packet_get_send_len(const struct dp_packet *b)
> > > +dp_packet_size(const struct dp_packet *b)
> > >  {
> > > -    return dp_packet_size(b) - dp_packet_get_cutlen(b);
> > > +    return b->size_;
> > >  }
> > >
> > > -static inline void *
> > > -dp_packet_data(const struct dp_packet *b)
> > > +static inline void
> > > +dp_packet_set_size(struct dp_packet *b, uint32_t v)
> > >  {
> > > -    return __packet_data(b) != UINT16_MAX
> > > -           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> > > +    b->size_ = v;
> > > +}
> > > +
> > > +static inline uint16_t
> > > +__packet_data(const struct dp_packet *b) {
> > > +    return b->data_ofs;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_set_data(struct dp_packet *b, void *data)
> > > +__packet_set_data(struct dp_packet *b, uint16_t v)
> > >  {
> > > -    if (data) {
> > > -        __packet_set_data(b, (char *) data - (char *)
> dp_packet_base(b));
> > > -    } else {
> > > -        __packet_set_data(b, UINT16_MAX);
> > > -    }
> > > +    b->data_ofs = v;
> > > +}
> > > +
> > > +static inline uint16_t
> > > +dp_packet_get_allocated(const struct dp_packet *b) {
> > > +    return b->allocated_;
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_reset_packet(struct dp_packet *b, int off)
> > > +dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
> > >  {
> > > -    dp_packet_set_size(b, dp_packet_size(b) - off);
> > > -    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) +
> off));
> > > -    dp_packet_reset_offsets(b);
> > > +    b->allocated_ = s;
> > >  }
> > >
> > >  /* Returns the RSS hash of the packet 'p'.  Note that the returned
> > > value is @@ -579,130 +608,133 @@ dp_packet_reset_packet(struct
> > > dp_packet *b, int off)  static inline uint32_t
> > > dp_packet_get_rss_hash(struct dp_packet *p)  { -#ifdef DPDK_NETDEV
> > > -    return p->mbuf.hash.rss;
> > > -#else
> > >      return p->rss_hash;
> > > -#endif
> > >  }
> > >
> > >  static inline void
> > >  dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)  {
> > > -#ifdef DPDK_NETDEV
> > > -    p->mbuf.hash.rss = hash;
> > > -    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
> > > -#else
> > >      p->rss_hash = hash;
> > >      p->rss_hash_valid = true;
> > > -#endif
> > >  }
> > >
> > >  static inline bool
> > >  dp_packet_rss_valid(struct dp_packet *p)  { -#ifdef DPDK_NETDEV
> > > -    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
> > > -#else
> > >      return p->rss_hash_valid;
> > > -#endif
> > >  }
> > >
> > >  static inline void
> > > -dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
> > > +dp_packet_rss_invalidate(struct dp_packet *p)
> > >  {
> > > -#ifndef DPDK_NETDEV
> > >      p->rss_hash_valid = false;
> > > -#endif
> > >  }
> > >
> > >  static inline void
> > >  dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED)  {
> > > -#ifdef DPDK_NETDEV
> > > -    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
> > > -#endif
> > >  }
> > >
> > > -/* This initialization is needed for packets that do not come
> > > - * from DPDK interfaces, when vswitchd is built with --with-dpdk.
> > > - * The DPDK rte library will still otherwise manage the mbuf.
> > > - * We only need to initialize the mbuf ol_flags. */  static inline
> > > void  dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)  { -#ifdef
> > > DPDK_NETDEV
> > > -    p->mbuf.ol_flags = 0;
> > > -#endif
> > >  }
> > >
> > >  static inline bool
> > >  dp_packet_ip_checksum_valid(struct dp_packet *p OVS_UNUSED)  {
> > > -#ifdef DPDK_NETDEV
> > > -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > -            PKT_RX_IP_CKSUM_GOOD;
> > > -#else
> > >      return false;
> > > -#endif
> > >  }
> > >
> > >  static inline bool
> > >  dp_packet_ip_checksum_bad(struct dp_packet *p OVS_UNUSED)  {
> > > -#ifdef DPDK_NETDEV
> > > -    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
> > > -            PKT_RX_IP_CKSUM_BAD;
> > > -#else
> > >      return false;
> > > -#endif
> > >  }
> > >
> > >  static inline bool
> > >  dp_packet_l4_checksum_valid(struct dp_packet *p OVS_UNUSED)  {
> > > -#ifdef DPDK_NETDEV
> > > -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > -            PKT_RX_L4_CKSUM_GOOD;
> > > -#else
> > >      return false;
> > > -#endif
> > >  }
> > >
> > >  static inline bool
> > >  dp_packet_l4_checksum_bad(struct dp_packet *p OVS_UNUSED)  {
> > > -#ifdef DPDK_NETDEV
> > > -    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
> > > -            PKT_RX_L4_CKSUM_BAD;
> > > -#else
> > >      return false;
> > > -#endif
> > >  }
> > >
> > > -#ifdef DPDK_NETDEV
> > >  static inline void
> > > -reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
> > > +reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED)
> > >  {
> > > -    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD
> |
> > > -                          PKT_RX_IP_CKSUM_GOOD |
> PKT_RX_IP_CKSUM_BAD);
> > >  }
> > > -#else
> > > -#define reset_dp_packet_checksum_ol_flags(arg)
> > > -#endif
> > >
> > >  static inline bool
> > >  dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
> > >                          uint32_t *mark OVS_UNUSED)  { -#ifdef
> > > DPDK_NETDEV
> > > -    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
> > > -        *mark = p->mbuf.hash.fdir.hi;
> > > -        return true;
> > > -    }
> > > -#endif
> > >      return false;
> > >  }
> > > +#endif /* DPDK_NETDEV */
> > > +
> > > +static inline void
> > > +dp_packet_reset_cutlen(struct dp_packet *b) {
> > > +    b->cutlen = 0;
> > > +}
> > > +
> > > +static inline uint32_t
> > > +dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len) {
> > > +    if (max_len < ETH_HEADER_LEN) {
> > > +        max_len = ETH_HEADER_LEN;
> > > +    }
> > > +
> > > +    if (max_len >= dp_packet_size(b)) {
> > > +        b->cutlen = 0;
> > > +    } else {
> > > +        b->cutlen = dp_packet_size(b) - max_len;
> > > +    }
> > > +    return b->cutlen;
> > > +}
> > > +
> > > +static inline uint32_t
> > > +dp_packet_get_cutlen(const struct dp_packet *b) {
> > > +    /* Always in valid range if user uses dp_packet_set_cutlen. */
> > > +    return b->cutlen;
> > > +}
> > > +
> > > +static inline uint32_t
> > > +dp_packet_get_send_len(const struct dp_packet *b) {
> > > +    return dp_packet_size(b) - dp_packet_get_cutlen(b); }
> > > +
> > > +static inline void *
> > > +dp_packet_data(const struct dp_packet *b) {
> > > +    return __packet_data(b) != UINT16_MAX
> > > +           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
> > > +}
> > > +
> > > +static inline void
> > > +dp_packet_set_data(struct dp_packet *b, void *data) {
> > > +    if (data) {
> > > +        __packet_set_data(b, (char *) data - (char *)
> dp_packet_base(b));
> > > +    } else {
> > > +        __packet_set_data(b, UINT16_MAX);
> > > +    }
> > > +}
> > > +
> > > +static inline void
> > > +dp_packet_reset_packet(struct dp_packet *b, int off) {
> > > +    dp_packet_set_size(b, dp_packet_size(b) - off);
> > > +    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) +
> off));
> > > +    dp_packet_reset_offsets(b);
> > > +}
> > >
> > >  enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a
> > > batch. */
> > >
> > >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Ben Pfaff Oct. 8, 2018, 6:05 p.m. | #4
On Mon, Oct 08, 2018 at 05:39:04PM +0000, Stokes, Ian wrote:
> > For what it's worth, Ian, I'm hoping that you'll take a look at this and
> > decide whether to merge it.
> 
> Sure, thanks for flagging Ben, apologies for the delay, I've been on
> vacation the past 2 weeks but will have time to look at this now.

Oh, I forgot about the vacation, even though you told me about it.
Please forgive me for also re-raising the other issue I emailed about a
few minutes ago.
Stokes, Ian Oct. 10, 2018, 10:36 a.m. | #5
> There is already an ifdef DPDK_NETDEV block, so instead of checking on
> each and every function, move them to the right block.
> 
> No functional change.

Thanks for this Flavio, LGTM, will be part of this weeks pull request.

Thanks
Ian

Patch

diff --git a/lib/dp-packet.h b/lib/dp-packet.h
index ba91e5891..5b4c9c7a3 100644
--- a/lib/dp-packet.h
+++ b/lib/dp-packet.h
@@ -465,113 +465,142 @@  dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
 {
     b->mbuf.buf_len = s;
 }
-#else
-static inline void *
-dp_packet_base(const struct dp_packet *b)
+
+/* Returns the RSS hash of the packet 'p'.  Note that the returned value is
+ * correct only if 'dp_packet_rss_valid(p)' returns true */
+static inline uint32_t
+dp_packet_get_rss_hash(struct dp_packet *p)
 {
-    return b->base_;
+    return p->mbuf.hash.rss;
 }
 
 static inline void
-dp_packet_set_base(struct dp_packet *b, void *d)
+dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
 {
-    b->base_ = d;
+    p->mbuf.hash.rss = hash;
+    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
 }
 
-static inline uint32_t
-dp_packet_size(const struct dp_packet *b)
+static inline bool
+dp_packet_rss_valid(struct dp_packet *p)
 {
-    return b->size_;
+    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
 }
 
 static inline void
-dp_packet_set_size(struct dp_packet *b, uint32_t v)
+dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
 {
-    b->size_ = v;
 }
 
-static inline uint16_t
-__packet_data(const struct dp_packet *b)
+static inline void
+dp_packet_mbuf_rss_flag_reset(struct dp_packet *p)
 {
-    return b->data_ofs;
+    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
 }
 
+/* This initialization is needed for packets that do not come
+ * from DPDK interfaces, when vswitchd is built with --with-dpdk.
+ * The DPDK rte library will still otherwise manage the mbuf.
+ * We only need to initialize the mbuf ol_flags. */
 static inline void
-__packet_set_data(struct dp_packet *b, uint16_t v)
+dp_packet_mbuf_init(struct dp_packet *p)
 {
-    b->data_ofs = v;
+    p->mbuf.ol_flags = 0;
 }
 
-static inline uint16_t
-dp_packet_get_allocated(const struct dp_packet *b)
+static inline bool
+dp_packet_ip_checksum_valid(struct dp_packet *p)
 {
-    return b->allocated_;
+    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
+            PKT_RX_IP_CKSUM_GOOD;
 }
 
-static inline void
-dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
+static inline bool
+dp_packet_ip_checksum_bad(struct dp_packet *p)
 {
-    b->allocated_ = s;
+    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
+            PKT_RX_IP_CKSUM_BAD;
+}
+
+static inline bool
+dp_packet_l4_checksum_valid(struct dp_packet *p)
+{
+    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
+            PKT_RX_L4_CKSUM_GOOD;
+}
+
+static inline bool
+dp_packet_l4_checksum_bad(struct dp_packet *p)
+{
+    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
+            PKT_RX_L4_CKSUM_BAD;
 }
-#endif
 
 static inline void
-dp_packet_reset_cutlen(struct dp_packet *b)
+reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
 {
-    b->cutlen = 0;
+    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
+                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
 }
 
-static inline uint32_t
-dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
+static inline bool
+dp_packet_has_flow_mark(struct dp_packet *p, uint32_t *mark)
 {
-    if (max_len < ETH_HEADER_LEN) {
-        max_len = ETH_HEADER_LEN;
+    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
+        *mark = p->mbuf.hash.fdir.hi;
+        return true;
     }
 
-    if (max_len >= dp_packet_size(b)) {
-        b->cutlen = 0;
-    } else {
-        b->cutlen = dp_packet_size(b) - max_len;
-    }
-    return b->cutlen;
+    return false;
 }
 
-static inline uint32_t
-dp_packet_get_cutlen(const struct dp_packet *b)
+#else /* DPDK_NETDEV */
+static inline void *
+dp_packet_base(const struct dp_packet *b)
 {
-    /* Always in valid range if user uses dp_packet_set_cutlen. */
-    return b->cutlen;
+    return b->base_;
+}
+
+static inline void
+dp_packet_set_base(struct dp_packet *b, void *d)
+{
+    b->base_ = d;
 }
 
 static inline uint32_t
-dp_packet_get_send_len(const struct dp_packet *b)
+dp_packet_size(const struct dp_packet *b)
 {
-    return dp_packet_size(b) - dp_packet_get_cutlen(b);
+    return b->size_;
 }
 
-static inline void *
-dp_packet_data(const struct dp_packet *b)
+static inline void
+dp_packet_set_size(struct dp_packet *b, uint32_t v)
 {
-    return __packet_data(b) != UINT16_MAX
-           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
+    b->size_ = v;
+}
+
+static inline uint16_t
+__packet_data(const struct dp_packet *b)
+{
+    return b->data_ofs;
 }
 
 static inline void
-dp_packet_set_data(struct dp_packet *b, void *data)
+__packet_set_data(struct dp_packet *b, uint16_t v)
 {
-    if (data) {
-        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
-    } else {
-        __packet_set_data(b, UINT16_MAX);
-    }
+    b->data_ofs = v;
+}
+
+static inline uint16_t
+dp_packet_get_allocated(const struct dp_packet *b)
+{
+    return b->allocated_;
 }
 
 static inline void
-dp_packet_reset_packet(struct dp_packet *b, int off)
+dp_packet_set_allocated(struct dp_packet *b, uint16_t s)
 {
-    dp_packet_set_size(b, dp_packet_size(b) - off);
-    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
-    dp_packet_reset_offsets(b);
+    b->allocated_ = s;
 }
 
 /* Returns the RSS hash of the packet 'p'.  Note that the returned value is
@@ -579,130 +608,133 @@  dp_packet_reset_packet(struct dp_packet *b, int off)
 static inline uint32_t
 dp_packet_get_rss_hash(struct dp_packet *p)
 {
-#ifdef DPDK_NETDEV
-    return p->mbuf.hash.rss;
-#else
     return p->rss_hash;
-#endif
 }
 
 static inline void
 dp_packet_set_rss_hash(struct dp_packet *p, uint32_t hash)
 {
-#ifdef DPDK_NETDEV
-    p->mbuf.hash.rss = hash;
-    p->mbuf.ol_flags |= PKT_RX_RSS_HASH;
-#else
     p->rss_hash = hash;
     p->rss_hash_valid = true;
-#endif
 }
 
 static inline bool
 dp_packet_rss_valid(struct dp_packet *p)
 {
-#ifdef DPDK_NETDEV
-    return p->mbuf.ol_flags & PKT_RX_RSS_HASH;
-#else
     return p->rss_hash_valid;
-#endif
 }
 
 static inline void
-dp_packet_rss_invalidate(struct dp_packet *p OVS_UNUSED)
+dp_packet_rss_invalidate(struct dp_packet *p)
 {
-#ifndef DPDK_NETDEV
     p->rss_hash_valid = false;
-#endif
 }
 
 static inline void
 dp_packet_mbuf_rss_flag_reset(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    p->mbuf.ol_flags &= ~PKT_RX_RSS_HASH;
-#endif
 }
 
-/* This initialization is needed for packets that do not come
- * from DPDK interfaces, when vswitchd is built with --with-dpdk.
- * The DPDK rte library will still otherwise manage the mbuf.
- * We only need to initialize the mbuf ol_flags. */
 static inline void
 dp_packet_mbuf_init(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    p->mbuf.ol_flags = 0;
-#endif
 }
 
 static inline bool
 dp_packet_ip_checksum_valid(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
-            PKT_RX_IP_CKSUM_GOOD;
-#else
     return false;
-#endif
 }
 
 static inline bool
 dp_packet_ip_checksum_bad(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    return (p->mbuf.ol_flags & PKT_RX_IP_CKSUM_MASK) ==
-            PKT_RX_IP_CKSUM_BAD;
-#else
     return false;
-#endif
 }
 
 static inline bool
 dp_packet_l4_checksum_valid(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
-            PKT_RX_L4_CKSUM_GOOD;
-#else
     return false;
-#endif
 }
 
 static inline bool
 dp_packet_l4_checksum_bad(struct dp_packet *p OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    return (p->mbuf.ol_flags & PKT_RX_L4_CKSUM_MASK) ==
-            PKT_RX_L4_CKSUM_BAD;
-#else
     return false;
-#endif
 }
 
-#ifdef DPDK_NETDEV
 static inline void
-reset_dp_packet_checksum_ol_flags(struct dp_packet *p)
+reset_dp_packet_checksum_ol_flags(struct dp_packet *p OVS_UNUSED)
 {
-    p->mbuf.ol_flags &= ~(PKT_RX_L4_CKSUM_GOOD | PKT_RX_L4_CKSUM_BAD |
-                          PKT_RX_IP_CKSUM_GOOD | PKT_RX_IP_CKSUM_BAD);
 }
-#else
-#define reset_dp_packet_checksum_ol_flags(arg)
-#endif
 
 static inline bool
 dp_packet_has_flow_mark(struct dp_packet *p OVS_UNUSED,
                         uint32_t *mark OVS_UNUSED)
 {
-#ifdef DPDK_NETDEV
-    if (p->mbuf.ol_flags & PKT_RX_FDIR_ID) {
-        *mark = p->mbuf.hash.fdir.hi;
-        return true;
-    }
-#endif
     return false;
 }
+#endif /* DPDK_NETDEV */
+
+static inline void
+dp_packet_reset_cutlen(struct dp_packet *b)
+{
+    b->cutlen = 0;
+}
+
+static inline uint32_t
+dp_packet_set_cutlen(struct dp_packet *b, uint32_t max_len)
+{
+    if (max_len < ETH_HEADER_LEN) {
+        max_len = ETH_HEADER_LEN;
+    }
+
+    if (max_len >= dp_packet_size(b)) {
+        b->cutlen = 0;
+    } else {
+        b->cutlen = dp_packet_size(b) - max_len;
+    }
+    return b->cutlen;
+}
+
+static inline uint32_t
+dp_packet_get_cutlen(const struct dp_packet *b)
+{
+    /* Always in valid range if user uses dp_packet_set_cutlen. */
+    return b->cutlen;
+}
+
+static inline uint32_t
+dp_packet_get_send_len(const struct dp_packet *b)
+{
+    return dp_packet_size(b) - dp_packet_get_cutlen(b);
+}
+
+static inline void *
+dp_packet_data(const struct dp_packet *b)
+{
+    return __packet_data(b) != UINT16_MAX
+           ? (char *) dp_packet_base(b) + __packet_data(b) : NULL;
+}
+
+static inline void
+dp_packet_set_data(struct dp_packet *b, void *data)
+{
+    if (data) {
+        __packet_set_data(b, (char *) data - (char *) dp_packet_base(b));
+    } else {
+        __packet_set_data(b, UINT16_MAX);
+    }
+}
+
+static inline void
+dp_packet_reset_packet(struct dp_packet *b, int off)
+{
+    dp_packet_set_size(b, dp_packet_size(b) - off);
+    dp_packet_set_data(b, ((unsigned char *) dp_packet_data(b) + off));
+    dp_packet_reset_offsets(b);
+}
 
 enum { NETDEV_MAX_BURST = 32 }; /* Maximum number packets in a batch. */