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

Stokes, Ian ian.stokes at intel.com
Mon Oct 8 17:39:04 UTC 2018


> 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 at 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 at 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 at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list