[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