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

Lam, Tiago tiago.lam at intel.com
Tue Oct 2 09:32:40 UTC 2018


(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. */
>  
> 


More information about the dev mailing list