| Message ID | 20171101205152.4827-1-blp@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev] nsh: Fix packet format to match IETF NSH draft. | expand |
On Thu, Nov 02, 2017 at 04:51:52AM +0800, Ben Pfaff wrote: > The NSH draft added a TTL field. This adds basic support. > > CC: Yi Yang <yi.y.yang@intel.com> > CC: Jan Scheurich <jan.scheurich@ericsson.com> > Signed-off-by: Ben Pfaff <blp@ovn.org> > --- > include/openvswitch/nsh.h | 11 +++++++---- > lib/flow.c | 2 +- > lib/odp-execute.c | 10 +++++----- > lib/packets.c | 4 +++- > 4 files changed, 16 insertions(+), 11 deletions(-) > > diff --git a/include/openvswitch/nsh.h b/include/openvswitch/nsh.h > index a3611d0896b2..4d83bad1af81 100644 > --- a/include/openvswitch/nsh.h > +++ b/include/openvswitch/nsh.h > @@ -62,7 +62,7 @@ struct nsh_md2_tlv { > }; > > struct nsh_hdr { > - ovs_be16 ver_flags_len; > + ovs_be16 ver_flags_ttl_len; > uint8_t md_type; > uint8_t next_proto; > ovs_16aligned_be32 path_hdr; > @@ -75,8 +75,10 @@ struct nsh_hdr { > /* Masking NSH header fields. */ > #define NSH_VER_MASK 0xc000 > #define NSH_VER_SHIFT 14 > -#define NSH_FLAGS_MASK 0x3fc0 > -#define NSH_FLAGS_SHIFT 6 > +#define NSH_FLAGS_MASK 0x3000 > +#define NSH_FLAGS_SHIFT 12 > +#define NSH_TTL_MASK 0x0fc0 > +#define NSH_TTL_SHIFT 6 > #define NSH_LEN_MASK 0x003f > #define NSH_LEN_SHIFT 0 > > @@ -113,7 +115,8 @@ struct nsh_hdr { > static inline uint16_t > nsh_hdr_len(const struct nsh_hdr *nsh) > { > - return ((ntohs(nsh->ver_flags_len) & NSH_LEN_MASK) >> NSH_LEN_SHIFT) << 2; > + return ((ntohs(nsh->ver_flags_ttl_len) & NSH_LEN_MASK) > + >> NSH_LEN_SHIFT) << 2; > } > > static inline struct nsh_md1_ctx * > diff --git a/lib/flow.c b/lib/flow.c > index 4d2b7747a124..57b6c597d207 100644 > --- a/lib/flow.c > +++ b/lib/flow.c > @@ -546,7 +546,7 @@ parse_nsh(const void **datap, size_t *sizep, struct flow_nsh *key) > > memset(key, 0, sizeof(struct flow_nsh)); > > - ver_flags_len = ntohs(nsh->ver_flags_len); Rename ver_flags_len to ver_flags_ttl_len. Do we need to consider new ttl key in this patch? > + ver_flags_len = ntohs(nsh->ver_flags_ttl_len); > version = (ver_flags_len & NSH_VER_MASK) >> NSH_VER_SHIFT; > flags = (ver_flags_len & NSH_FLAGS_MASK) >> NSH_FLAGS_SHIFT; > > diff --git a/lib/odp-execute.c b/lib/odp-execute.c > index 5f4d23a91a3e..d5542bde8b02 100644 > --- a/lib/odp-execute.c > +++ b/lib/odp-execute.c > @@ -279,8 +279,8 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key, > struct nsh_hdr *nsh = dp_packet_l3(packet); > > if (!mask) { > - nsh->ver_flags_len = htons(key->flags << NSH_FLAGS_SHIFT) | > - (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK)); > + nsh->ver_flags_ttl_len = htons(key->flags << NSH_FLAGS_SHIFT) | > + (nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK)); > put_16aligned_be32(&nsh->path_hdr, key->path_hdr); > switch (nsh->md_type) { > case NSH_M_TYPE1: > @@ -294,11 +294,11 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key, > break; > } > } else { > - uint8_t flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >> > + uint8_t flags = (ntohs(nsh->ver_flags_ttl_len) & NSH_FLAGS_MASK) >> > NSH_FLAGS_SHIFT; > flags = key->flags | (flags & ~mask->flags); > - nsh->ver_flags_len = htons(flags << NSH_FLAGS_SHIFT) | > - (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK)); > + nsh->ver_flags_ttl_len = htons(flags << NSH_FLAGS_SHIFT) | > + (nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK)); > > ovs_be32 path_hdr = get_16aligned_be32(&nsh->path_hdr); > path_hdr = key->path_hdr | (path_hdr & ~mask->path_hdr); > diff --git a/lib/packets.c b/lib/packets.c > index 74d87eda89e1..a26b6c33e41c 100644 > --- a/lib/packets.c > +++ b/lib/packets.c > @@ -427,7 +427,9 @@ encap_nsh(struct dp_packet *packet, const struct ovs_action_encap_nsh *encap) > } > > nsh = (struct nsh_hdr *) dp_packet_push_uninit(packet, length); > - nsh->ver_flags_len = htons(encap->flags << NSH_FLAGS_SHIFT | length >> 2); > + nsh->ver_flags_ttl_len = htons((encap->flags << NSH_FLAGS_SHIFT) > + | (63 << NSH_TTL_SHIFT) > + | ((length >> 2) << NSH_LEN_SHIFT)); > nsh->next_proto = next_proto; > put_16aligned_be32(&nsh->path_hdr, encap->path_hdr); > nsh->md_type = encap->mdtype; > -- > 2.10.2 >
On Mon, Nov 06, 2017 at 06:51:57PM +0800, Yang, Yi wrote: > On Thu, Nov 02, 2017 at 04:51:52AM +0800, Ben Pfaff wrote: > > The NSH draft added a TTL field. This adds basic support. > > > > CC: Yi Yang <yi.y.yang@intel.com> > > CC: Jan Scheurich <jan.scheurich@ericsson.com> > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > --- > > include/openvswitch/nsh.h | 11 +++++++---- > > lib/flow.c | 2 +- > > lib/odp-execute.c | 10 +++++----- > > lib/packets.c | 4 +++- > > 4 files changed, 16 insertions(+), 11 deletions(-) > > > > diff --git a/include/openvswitch/nsh.h b/include/openvswitch/nsh.h > > index a3611d0896b2..4d83bad1af81 100644 > > --- a/include/openvswitch/nsh.h > > +++ b/include/openvswitch/nsh.h > > @@ -62,7 +62,7 @@ struct nsh_md2_tlv { > > }; > > > > struct nsh_hdr { > > - ovs_be16 ver_flags_len; > > + ovs_be16 ver_flags_ttl_len; > > uint8_t md_type; > > uint8_t next_proto; > > ovs_16aligned_be32 path_hdr; > > @@ -75,8 +75,10 @@ struct nsh_hdr { > > /* Masking NSH header fields. */ > > #define NSH_VER_MASK 0xc000 > > #define NSH_VER_SHIFT 14 > > -#define NSH_FLAGS_MASK 0x3fc0 > > -#define NSH_FLAGS_SHIFT 6 > > +#define NSH_FLAGS_MASK 0x3000 > > +#define NSH_FLAGS_SHIFT 12 > > +#define NSH_TTL_MASK 0x0fc0 > > +#define NSH_TTL_SHIFT 6 > > #define NSH_LEN_MASK 0x003f > > #define NSH_LEN_SHIFT 0 > > > > @@ -113,7 +115,8 @@ struct nsh_hdr { > > static inline uint16_t > > nsh_hdr_len(const struct nsh_hdr *nsh) > > { > > - return ((ntohs(nsh->ver_flags_len) & NSH_LEN_MASK) >> NSH_LEN_SHIFT) << 2; > > + return ((ntohs(nsh->ver_flags_ttl_len) & NSH_LEN_MASK) > > + >> NSH_LEN_SHIFT) << 2; > > } > > > > static inline struct nsh_md1_ctx * > > diff --git a/lib/flow.c b/lib/flow.c > > index 4d2b7747a124..57b6c597d207 100644 > > --- a/lib/flow.c > > +++ b/lib/flow.c > > @@ -546,7 +546,7 @@ parse_nsh(const void **datap, size_t *sizep, struct flow_nsh *key) > > > > memset(key, 0, sizeof(struct flow_nsh)); > > > > - ver_flags_len = ntohs(nsh->ver_flags_len); > > Rename ver_flags_len to ver_flags_ttl_len. > > Do we need to consider new ttl key in this patch? I assumed that at the very least we needed to set the TTL to something reasonable initially. This patch doesn't allow OVS to match on or to decrement the TTL, which certainly isn't ideal but still seems like a better situation than being entirely compatible at the wire level. I'll accept whatever you prefer, though.
diff --git a/include/openvswitch/nsh.h b/include/openvswitch/nsh.h index a3611d0896b2..4d83bad1af81 100644 --- a/include/openvswitch/nsh.h +++ b/include/openvswitch/nsh.h @@ -62,7 +62,7 @@ struct nsh_md2_tlv { }; struct nsh_hdr { - ovs_be16 ver_flags_len; + ovs_be16 ver_flags_ttl_len; uint8_t md_type; uint8_t next_proto; ovs_16aligned_be32 path_hdr; @@ -75,8 +75,10 @@ struct nsh_hdr { /* Masking NSH header fields. */ #define NSH_VER_MASK 0xc000 #define NSH_VER_SHIFT 14 -#define NSH_FLAGS_MASK 0x3fc0 -#define NSH_FLAGS_SHIFT 6 +#define NSH_FLAGS_MASK 0x3000 +#define NSH_FLAGS_SHIFT 12 +#define NSH_TTL_MASK 0x0fc0 +#define NSH_TTL_SHIFT 6 #define NSH_LEN_MASK 0x003f #define NSH_LEN_SHIFT 0 @@ -113,7 +115,8 @@ struct nsh_hdr { static inline uint16_t nsh_hdr_len(const struct nsh_hdr *nsh) { - return ((ntohs(nsh->ver_flags_len) & NSH_LEN_MASK) >> NSH_LEN_SHIFT) << 2; + return ((ntohs(nsh->ver_flags_ttl_len) & NSH_LEN_MASK) + >> NSH_LEN_SHIFT) << 2; } static inline struct nsh_md1_ctx * diff --git a/lib/flow.c b/lib/flow.c index 4d2b7747a124..57b6c597d207 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -546,7 +546,7 @@ parse_nsh(const void **datap, size_t *sizep, struct flow_nsh *key) memset(key, 0, sizeof(struct flow_nsh)); - ver_flags_len = ntohs(nsh->ver_flags_len); + ver_flags_len = ntohs(nsh->ver_flags_ttl_len); version = (ver_flags_len & NSH_VER_MASK) >> NSH_VER_SHIFT; flags = (ver_flags_len & NSH_FLAGS_MASK) >> NSH_FLAGS_SHIFT; diff --git a/lib/odp-execute.c b/lib/odp-execute.c index 5f4d23a91a3e..d5542bde8b02 100644 --- a/lib/odp-execute.c +++ b/lib/odp-execute.c @@ -279,8 +279,8 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key, struct nsh_hdr *nsh = dp_packet_l3(packet); if (!mask) { - nsh->ver_flags_len = htons(key->flags << NSH_FLAGS_SHIFT) | - (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK)); + nsh->ver_flags_ttl_len = htons(key->flags << NSH_FLAGS_SHIFT) | + (nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK)); put_16aligned_be32(&nsh->path_hdr, key->path_hdr); switch (nsh->md_type) { case NSH_M_TYPE1: @@ -294,11 +294,11 @@ odp_set_nsh(struct dp_packet *packet, const struct ovs_key_nsh *key, break; } } else { - uint8_t flags = (ntohs(nsh->ver_flags_len) & NSH_FLAGS_MASK) >> + uint8_t flags = (ntohs(nsh->ver_flags_ttl_len) & NSH_FLAGS_MASK) >> NSH_FLAGS_SHIFT; flags = key->flags | (flags & ~mask->flags); - nsh->ver_flags_len = htons(flags << NSH_FLAGS_SHIFT) | - (nsh->ver_flags_len & ~htons(NSH_FLAGS_MASK)); + nsh->ver_flags_ttl_len = htons(flags << NSH_FLAGS_SHIFT) | + (nsh->ver_flags_ttl_len & ~htons(NSH_FLAGS_MASK)); ovs_be32 path_hdr = get_16aligned_be32(&nsh->path_hdr); path_hdr = key->path_hdr | (path_hdr & ~mask->path_hdr); diff --git a/lib/packets.c b/lib/packets.c index 74d87eda89e1..a26b6c33e41c 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -427,7 +427,9 @@ encap_nsh(struct dp_packet *packet, const struct ovs_action_encap_nsh *encap) } nsh = (struct nsh_hdr *) dp_packet_push_uninit(packet, length); - nsh->ver_flags_len = htons(encap->flags << NSH_FLAGS_SHIFT | length >> 2); + nsh->ver_flags_ttl_len = htons((encap->flags << NSH_FLAGS_SHIFT) + | (63 << NSH_TTL_SHIFT) + | ((length >> 2) << NSH_LEN_SHIFT)); nsh->next_proto = next_proto; put_16aligned_be32(&nsh->path_hdr, encap->path_hdr); nsh->md_type = encap->mdtype;
The NSH draft added a TTL field. This adds basic support. CC: Yi Yang <yi.y.yang@intel.com> CC: Jan Scheurich <jan.scheurich@ericsson.com> Signed-off-by: Ben Pfaff <blp@ovn.org> --- include/openvswitch/nsh.h | 11 +++++++---- lib/flow.c | 2 +- lib/odp-execute.c | 10 +++++----- lib/packets.c | 4 +++- 4 files changed, 16 insertions(+), 11 deletions(-)