[ovs-dev] [PATCH 12/12] Expand tunnel IDs from 32 to 64 bits.

Jesse Gross jesse at nicira.com
Thu Dec 9 05:15:59 UTC 2010


On Tue, Dec 7, 2010 at 11:00 AM, Ben Pfaff <blp at nicira.com> wrote:
>  static inline struct tnl_vport *tnl_vport_priv(const struct vport *vport)
> diff --git a/datapath/vport-gre.c b/datapath/vport-gre.c
> index f6996ed..b7e256c 100644
> --- a/datapath/vport-gre.c
> +++ b/datapath/vport-gre.c
> @@ -71,7 +71,7 @@ static void gre_build_header(const struct vport *vport,
>                greh->flags |= GRE_KEY;
>
>        if (mutable->port_config.out_key)
> -               *options = mutable->port_config.out_key;
> +               *options = htonl(be64_to_cpu(mutable->port_config.out_key));
>  }

We should be able to do better than a double byte swap to handle
truncation.  It's not so big a deal here, when we are just building up
the cache but in gre_update_header() and parse header() it matters.
We could just replace this with a simple shift on little endian
platforms and a no-op on big endian.

>
>  static struct sk_buff *gre_update_header(const struct vport *vport,
> @@ -84,7 +84,7 @@ static struct sk_buff *gre_update_header(const struct vport *vport,
>
>        /* Work backwards over the options so the checksum is last. */
>        if (mutable->port_config.flags & TNL_F_OUT_KEY_ACTION) {
> -               *options = OVS_CB(skb)->tun_id;
> +               *options = htonl(OVS_CB(skb)->tun_id);
>                options--;

This byte swap isn't right: tun_id isn't in host byte order.

> diff --git a/include/openvswitch/tunnel.h b/include/openvswitch/tunnel.h
> index e7d3fce..c338cd0 100644
> --- a/include/openvswitch/tunnel.h
> +++ b/include/openvswitch/tunnel.h
> @@ -53,13 +53,13 @@
>
>  /* This goes in the "config" member of struct odp_port for tunnel vports. */
>  struct tnl_port_config {
> -       __u32   flags;
> -       __be32  saddr;
> -       __be32  daddr;
> -       __be32  in_key;
> -       __be32  out_key;
> -       __u8    tos;
> -       __u8    ttl;
> +       __aligned_u64   in_key;
> +       __aligned_u64   out_key;

__aligned_be64?

> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index ea31c79..b63ab86 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -318,7 +318,7 @@ parse_nxm_entry(struct cls_rule *rule, const struct nxm_field *f,
>
>         /* Tunnel ID. */
>     case NFI_NXM_NX_TUN_ID:
> -        flow->tun_id = htonl(ntohll(get_unaligned_be64(value)));
> +        flow->tun_id = ntohll(get_unaligned_be64(value));
>         return 0;
>
>         /* Registers. */
> @@ -646,7 +646,7 @@ nx_put_match(struct ofpbuf *b, const struct cls_rule *cr)
>
>     /* Tunnel ID. */
>     if (!(wc & FWW_TUN_ID)) {
> -        nxm_put_64(b, NXM_NX_TUN_ID, htonll(ntohl(flow->tun_id)));
> +        nxm_put_64(b, NXM_NX_TUN_ID, htonll(flow->tun_id));
>     }

Why do we need to do any byte swapping here?  Shouldn't everything be
the same size and in network byte order?




More information about the dev mailing list