| Message ID | 1466070743-8933-1-git-send-email-johnson.li@intel.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Regards _Sugesh > -----Original Message----- > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Johnson Li > Sent: Thursday, June 16, 2016 10:52 AM > To: dev@openvswitch.org > Subject: [ovs-dev] [RFC PATCH 07/14] Add APIs to set NSH keys for match > fields > > Signed-off-by: Johnson Li <johnson.li@intel.com> > > diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h > index c955753..a1cca7e 100644 > --- a/include/openvswitch/match.h > +++ b/include/openvswitch/match.h > @@ -86,6 +86,26 @@ void match_set_tun_gbp_id_masked(struct match > *match, ovs_be16 gbp_id, ovs_be16 void match_set_tun_gbp_id(struct > match *match, ovs_be16 gbp_id); void > match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, > uint8_t mask); void match_set_tun_gbp_flags(struct match *match, uint8_t > flags); > + > +void match_set_nsp_masked(struct match *, ovs_be32 nsp, ovs_be32 > mask); > +void match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t > +mask); void match_set_nsh_md_type_masked(struct match *match, > uint8_t md_type, > + uint8_t mask); void > +match_set_nsh_next_proto_masked(struct match *match, uint8_t > next_proto, > + uint8_t mask); void > +match_set_nshc1_masked(struct match *, ovs_be32 nshc1, ovs_be32 > mask); > +void match_set_nshc2_masked(struct match *, ovs_be32 nshc2, ovs_be32 > +mask); void match_set_nshc3_masked(struct match *, ovs_be32 nshc3, > +ovs_be32 mask); void match_set_nshc4_masked(struct match *, ovs_be32 > +nshc4, ovs_be32 mask); void match_set_nsp(struct match *, ovs_be32 > +nsp); void match_set_nsi(struct match *match, uint8_t nsi); void > +match_set_nsh_md_type(struct match *match, uint8_t md_type); void > +match_set_nsh_next_proto(struct match *match, uint8_t next_proto); > void > +match_set_nshc1(struct match *, ovs_be32 nshc1); void > +match_set_nshc2(struct match *, ovs_be32 nshc2); void > +match_set_nshc3(struct match *, ovs_be32 nshc3); void > +match_set_nshc4(struct match *, ovs_be32 nshc4); [Sugesh] I feel, match_set for every nsh field is a overkill especially when we implement NSH type-2? Can we do this some more optimized way? Consider the MD2 type as well when defining the match_set definitions. > + > void match_set_in_port(struct match *, ofp_port_t ofp_port); void > match_set_pkt_mark(struct match *, uint32_t pkt_mark); void > match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t > mask); diff --git a/lib/match.c b/lib/match.c index f12c802..c0cc165 100644 > --- a/lib/match.c > +++ b/lib/match.c > @@ -837,6 +837,112 @@ match_set_nd_target_masked(struct match > *match, > match->wc.masks.nd_target = *mask; > } > > +void > +match_set_nsp_masked(struct match *match, ovs_be32 nsp, ovs_be32 > mask) > +{ > + match->wc.masks.nsh.nsp = mask; > + match->flow.nsh.nsp = nsp & mask; > +} > + > +void > +match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t mask) { > + match->wc.masks.nsh.nsi = mask; > + match->flow.nsh.nsi = nsi & mask; > +} > + > +void > +match_set_nsh_md_type_masked(struct match *match, > + uint8_t md_type, uint8_t mask) > +{ > + match->wc.masks.nsh.md_type = mask; > + match->flow.nsh.md_type = md_type & mask; } > + > +void > +match_set_nsh_next_proto_masked(struct match *match, > + uint8_t next_proto, uint8_t mask) { > + match->wc.masks.nsh.next_proto = mask; > + match->flow.nsh.next_proto = next_proto & mask; } > + > +void > +match_set_nshc1_masked(struct match *match, ovs_be32 nshc1, > ovs_be32 > +mask) { > + match->wc.masks.nsh.nshc1 = mask; > + match->flow.nsh.nshc1 = nshc1 & mask; } > + > +void > +match_set_nshc2_masked(struct match *match, ovs_be32 nshc2, > ovs_be32 > +mask) { > + match->wc.masks.nsh.nshc2 = mask; > + match->flow.nsh.nshc2 = nshc2 & mask; } > + > +void > +match_set_nshc3_masked(struct match *match, ovs_be32 nshc3, > ovs_be32 > +mask) { > + match->wc.masks.nsh.nshc3 = mask; > + match->flow.nsh.nshc3 = nshc3 & mask; } > + > +void > +match_set_nshc4_masked(struct match *match, ovs_be32 nshc4, > ovs_be32 > +mask) { > + match->wc.masks.nsh.nshc4 = mask; > + match->flow.nsh.nshc4 = nshc4 & mask; } > + > +void > +match_set_nsp(struct match *match, ovs_be32 nsp) { > + match_set_nsp_masked(match, nsp, OVS_BE32_MAX); } > + > +void > +match_set_nsi(struct match *match, uint8_t nsi) { > + match_set_nsi_masked(match, nsi, UINT8_MAX); } > + > +void > +match_set_nsh_md_type(struct match *match, uint8_t md_type) { > + match_set_nsh_md_type_masked(match, md_type, UINT8_MAX); } > + > +void > +match_set_nsh_next_proto(struct match *match, uint8_t next_proto) { > + match_set_nsh_next_proto_masked(match, next_proto, UINT8_MAX); } > + > +void > +match_set_nshc1(struct match *match, ovs_be32 nshc1) { > + match_set_nshc1_masked(match, nshc1, OVS_BE32_MAX); } > + > +void > +match_set_nshc2(struct match *match, ovs_be32 nshc2) { > + match_set_nshc2_masked(match, nshc2, OVS_BE32_MAX); } > + > +void > +match_set_nshc3(struct match *match, ovs_be32 nshc3) { > + match_set_nshc3_masked(match, nshc3, OVS_BE32_MAX); } > + > +void > +match_set_nshc4(struct match *match, ovs_be32 nshc4) { > + match_set_nshc4_masked(match, nshc4, OVS_BE32_MAX); } > + > /* Returns true if 'a' and 'b' wildcard the same fields and have the same > * values for fixed fields, otherwise false. */ bool > -- > 1.8.4.2 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
> > Regards > _Sugesh > > > > -----Original Message----- > > From: dev [mailto:dev-bounces@openvswitch.org] On Behalf Of Johnson Li > > Sent: Thursday, June 16, 2016 10:52 AM > > To: dev@openvswitch.org > > Subject: [ovs-dev] [RFC PATCH 07/14] Add APIs to set NSH keys for > > match fields > > > > Signed-off-by: Johnson Li <johnson.li@intel.com> > > > > diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h > > index c955753..a1cca7e 100644 > > --- a/include/openvswitch/match.h > > +++ b/include/openvswitch/match.h > > @@ -86,6 +86,26 @@ void match_set_tun_gbp_id_masked(struct match > > *match, ovs_be16 gbp_id, ovs_be16 void match_set_tun_gbp_id(struct > > match *match, ovs_be16 gbp_id); void > > match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, > > uint8_t mask); void match_set_tun_gbp_flags(struct match *match, > > uint8_t flags); > > + > > +void match_set_nsp_masked(struct match *, ovs_be32 nsp, ovs_be32 > > mask); > > +void match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t > > +mask); void match_set_nsh_md_type_masked(struct match *match, > > uint8_t md_type, > > + uint8_t mask); void > > +match_set_nsh_next_proto_masked(struct match *match, uint8_t > > next_proto, > > + uint8_t mask); void > > +match_set_nshc1_masked(struct match *, ovs_be32 nshc1, ovs_be32 > > mask); > > +void match_set_nshc2_masked(struct match *, ovs_be32 nshc2, > ovs_be32 > > +mask); void match_set_nshc3_masked(struct match *, ovs_be32 nshc3, > > +ovs_be32 mask); void match_set_nshc4_masked(struct match *, > ovs_be32 > > +nshc4, ovs_be32 mask); void match_set_nsp(struct match *, ovs_be32 > > +nsp); void match_set_nsi(struct match *match, uint8_t nsi); void > > +match_set_nsh_md_type(struct match *match, uint8_t md_type); void > > +match_set_nsh_next_proto(struct match *match, uint8_t next_proto); > > void > > +match_set_nshc1(struct match *, ovs_be32 nshc1); void > > +match_set_nshc2(struct match *, ovs_be32 nshc2); void > > +match_set_nshc3(struct match *, ovs_be32 nshc3); void > > +match_set_nshc4(struct match *, ovs_be32 nshc4); > [Sugesh] I feel, match_set for every nsh field is a overkill especially when we > implement NSH type-2? Can we do this some more optimized way? > Consider the MD2 type as well when defining the match_set definitions. > [JL] I will try to use MACROs to do this. For MD type 2, if all agree to reuse the Tun_metadata[0...63], then there is no need to add new fields and APIs. > > + > > void match_set_in_port(struct match *, ofp_port_t ofp_port); void > > match_set_pkt_mark(struct match *, uint32_t pkt_mark); void > > match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t > > mask); diff --git a/lib/match.c b/lib/match.c index f12c802..c0cc165 > > 100644 > > --- a/lib/match.c > > +++ b/lib/match.c > > @@ -837,6 +837,112 @@ match_set_nd_target_masked(struct match > *match, > > match->wc.masks.nd_target = *mask; } > > > > +void > > +match_set_nsp_masked(struct match *match, ovs_be32 nsp, ovs_be32 > > mask) > > +{ > > + match->wc.masks.nsh.nsp = mask; > > + match->flow.nsh.nsp = nsp & mask; } > > + > > +void > > +match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t mask) { > > + match->wc.masks.nsh.nsi = mask; > > + match->flow.nsh.nsi = nsi & mask; } > > + > > +void > > +match_set_nsh_md_type_masked(struct match *match, > > + uint8_t md_type, uint8_t mask) { > > + match->wc.masks.nsh.md_type = mask; > > + match->flow.nsh.md_type = md_type & mask; } > > + > > +void > > +match_set_nsh_next_proto_masked(struct match *match, > > + uint8_t next_proto, uint8_t mask) { > > + match->wc.masks.nsh.next_proto = mask; > > + match->flow.nsh.next_proto = next_proto & mask; } > > + > > +void > > +match_set_nshc1_masked(struct match *match, ovs_be32 nshc1, > > ovs_be32 > > +mask) { > > + match->wc.masks.nsh.nshc1 = mask; > > + match->flow.nsh.nshc1 = nshc1 & mask; } > > + > > +void > > +match_set_nshc2_masked(struct match *match, ovs_be32 nshc2, > > ovs_be32 > > +mask) { > > + match->wc.masks.nsh.nshc2 = mask; > > + match->flow.nsh.nshc2 = nshc2 & mask; } > > + > > +void > > +match_set_nshc3_masked(struct match *match, ovs_be32 nshc3, > > ovs_be32 > > +mask) { > > + match->wc.masks.nsh.nshc3 = mask; > > + match->flow.nsh.nshc3 = nshc3 & mask; } > > + > > +void > > +match_set_nshc4_masked(struct match *match, ovs_be32 nshc4, > > ovs_be32 > > +mask) { > > + match->wc.masks.nsh.nshc4 = mask; > > + match->flow.nsh.nshc4 = nshc4 & mask; } > > + > > +void > > +match_set_nsp(struct match *match, ovs_be32 nsp) { > > + match_set_nsp_masked(match, nsp, OVS_BE32_MAX); } > > + > > +void > > +match_set_nsi(struct match *match, uint8_t nsi) { > > + match_set_nsi_masked(match, nsi, UINT8_MAX); } > > + > > +void > > +match_set_nsh_md_type(struct match *match, uint8_t md_type) { > > + match_set_nsh_md_type_masked(match, md_type, UINT8_MAX); } > > + > > +void > > +match_set_nsh_next_proto(struct match *match, uint8_t next_proto) { > > + match_set_nsh_next_proto_masked(match, next_proto, > UINT8_MAX); } > > + > > +void > > +match_set_nshc1(struct match *match, ovs_be32 nshc1) { > > + match_set_nshc1_masked(match, nshc1, OVS_BE32_MAX); } > > + > > +void > > +match_set_nshc2(struct match *match, ovs_be32 nshc2) { > > + match_set_nshc2_masked(match, nshc2, OVS_BE32_MAX); } > > + > > +void > > +match_set_nshc3(struct match *match, ovs_be32 nshc3) { > > + match_set_nshc3_masked(match, nshc3, OVS_BE32_MAX); } > > + > > +void > > +match_set_nshc4(struct match *match, ovs_be32 nshc4) { > > + match_set_nshc4_masked(match, nshc4, OVS_BE32_MAX); } > > + > > /* Returns true if 'a' and 'b' wildcard the same fields and have the same > > * values for fixed fields, otherwise false. */ bool > > -- > > 1.8.4.2 > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > http://openvswitch.org/mailman/listinfo/dev
> > > > > Regards > > _Sugesh > > > > > > [Sugesh] I feel, match_set for every nsh field is a overkill > > especially when we implement NSH type-2? Can we do this some more > optimized way? > > Consider the MD2 type as well when defining the match_set definitions. > > > [JL] I will try to use MACROs to do this. For MD type 2, if all agree to reuse > the Tun_metadata[0...63], then there is no need to add new fields and APIs. [JL] Hi all, I'm going to prepare a new version of patch set with the following changes: 1) converge the comments by far and follow the comments to improve the Quality of the codes. 2) Add framework codes (or full implementation) for MD type 2 support of the NSH header. According to the discussion at http://comments.gmane.org/gmane.network.openvswitch.devel/53788 I will reuse the definitions for METADATA[0...63]. Since NSH is not one of The tunneling encapsulation headers, renaming for the fields is required. This might be a huge change for the current code base: a) command line will be changed: no field for the md_type is set in within command line. Once NSHCx is set within command line, md_type equals 1; md_type equals 2 only incase the NSHCx are not set and at least one of METADATAx is set. b) openflow message will be parsed with different manners: both geneve and NSH headers use the METADATAn fields, so parsing and formatting these fields are different from current mode. c) header parsing/building will take care of these fields. I have some concern about the compatibility issue, we need to make tradeoff. seems there is no better solutions so far. 3) Add "Ethernet + NSH" support for the OVS. Following the discussion at http://comments.gmane.org/gmane.network.openvswitch.devel/53788 we will introduce new match fields "encap_eth_dst/encap_eth_src" and related openflow actions"push_encap_eth/pop_encap_eth". Since we treat outer Ethernet header as encapsulation header, so NO VLAN and MPLS tags are supported for the outer header. That means packets with Format "Ethernet + VLAN[|MPLS] + NSH + Ethernet + IPv4[|IPv6] + playload" Will be dropped by the OVS when parsing the packets. Examples for Ethernet NSH case: 1) encapsulation: #sudo ovs-ofctl add-flow ovsbr "in_port=2, actions=set_field:1->nsp, \ set_field: 255->nsi, push_nsh, set_field: 00:11:22:33:44:55->encap_eth_dst, \ push_encap_eth, output: 3" 2) decapsulation: #sudo ovs-ofctl add-flow ovsbr "in_port=3, nsp=0x800001, nsi=253, \ encap_eth_src=00:11:22:33:44:55, actions=pop_encap_eth, pop_nsh, output: 3" If there is no opposed comments for the MD type 2 and "Ethernet + NSH" design, I will try to send out new version of patch ASAP. If you have any better advice For the design, please raise your voice. If you have different opinions with the discussion At http://comments.gmane.org/gmane.network.openvswitch.devel/53788. We May need to make agreement with You, Jesse and me first then I can send out The patch. Thanks! > > > -- > > > 1.8.4.2 > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > http://openvswitch.org/mailman/listinfo/dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On Mon, Jun 20, 2016 at 9:02 PM, Li, Johnson <johnson.li@intel.com> wrote: > 2) Add framework codes (or full implementation) for MD type 2 support of > the NSH header. According to the discussion at > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > I will reuse the definitions for METADATA[0...63]. Since NSH is not one of > The tunneling encapsulation headers, renaming for the fields is required. > This might be a huge change for the current code base: Please see this discussion with you coworker from 6 months ago: http://openvswitch.org/pipermail/dev/2015-December/063306.html You guys already renamed the fields once. At this point, you need to live with what is there since we're not going to break compatibility with previous releases. > 3) Add "Ethernet + NSH" support for the OVS. Following the discussion at > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > we will introduce new match fields "encap_eth_dst/encap_eth_src" and > related openflow actions"push_encap_eth/pop_encap_eth". Since we > treat outer Ethernet header as encapsulation header, so NO VLAN and > MPLS tags are supported for the outer header. That means packets with > Format "Ethernet + VLAN[|MPLS] + NSH + Ethernet + IPv4[|IPv6] + playload" > Will be dropped by the OVS when parsing the packets. > Examples for Ethernet NSH case: Please make a generalized push/pop Ethernet header rather than a specialized one for this case. There are other use cases that involve this and they should be consistent (i.e. connecting an L3 tunnel to an L2 interface, MAC-in-MAC encapsulation, etc.)
On Tue, Jun 21, 2016 at 02:08:34PM -0700, Jesse Gross wrote: > On Mon, Jun 20, 2016 at 9:02 PM, Li, Johnson <johnson.li@intel.com> wrote: > > 2) Add framework codes (or full implementation) for MD type 2 support of > > the NSH header. According to the discussion at > > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > > I will reuse the definitions for METADATA[0...63]. Since NSH is not one of > > The tunneling encapsulation headers, renaming for the fields is required. > > This might be a huge change for the current code base: > > Please see this discussion with you coworker from 6 months ago: > http://openvswitch.org/pipermail/dev/2015-December/063306.html > > You guys already renamed the fields once. At this point, you need to > live with what is there since we're not going to break compatibility > with previous releases. > > > 3) Add "Ethernet + NSH" support for the OVS. Following the discussion at > > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > > we will introduce new match fields "encap_eth_dst/encap_eth_src" and > > related openflow actions"push_encap_eth/pop_encap_eth". Since we > > treat outer Ethernet header as encapsulation header, so NO VLAN and > > MPLS tags are supported for the outer header. That means packets with > > Format "Ethernet + VLAN[|MPLS] + NSH + Ethernet + IPv4[|IPv6] + playload" > > Will be dropped by the OVS when parsing the packets. > > Examples for Ethernet NSH case: > > Please make a generalized push/pop Ethernet header rather than a > specialized one for this case. There are other use cases that involve > this and they should be consistent (i.e. connecting an L3 tunnel to an > L2 interface, MAC-in-MAC encapsulation, etc.) For "Eth + NSH + L2 Frame" case, we need ovs to parse inner Ethernet header and IP header, so we have to tell outer Ethernet header from inner Ethernet header, this is why we have to bring push_encap_eth and pop_encap_eth. Strictly speaking, Eth in "Eth + NSH + L2 Frame" is a new tunnel encapsulation, it is different from normal Ethernet header. > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
> On Mon, Jun 20, 2016 at 9:02 PM, Li, Johnson <johnson.li@intel.com> wrote: > > 2) Add framework codes (or full implementation) for MD type 2 support of > > the NSH header. According to the discussion at > > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > > I will reuse the definitions for METADATA[0...63]. Since NSH is not one of > > The tunneling encapsulation headers, renaming for the fields is required. > > This might be a huge change for the current code base: > > Please see this discussion with you coworker from 6 months ago: > http://openvswitch.org/pipermail/dev/2015-December/063306.html > > You guys already renamed the fields once. At this point, you need to live with > what is there since we're not going to break compatibility with previous releases. > [JL] Yes, Mengke had renamed the command line and mapping table from xx-geneve-yy To xx-tlv-yy. However, the flow fields are still named TUN_xxxx and bind to geneve Tunneling ports. So there are some issues here. For example, I added one VxLAN port With the following command: # sudo ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 type=vxlan \ options:dst_port=4790 options:remote_ip=192.168.50.101 \ options:key=1000 options:exts=gpe Then I tried to add some TLV map and then some flows: #sudo ovs-ofctl add-tlv-map br-int "{class=0xffff,type=0,len=4}->tun_metadata0, \ {class=0xffff,type=1,len=8}->tun_metadata1" #sudo ovs-ofctl add-flow br-int "table=0, priority=260, in_port=LOCAL \ actions=load:0x11223344->NXM_NX_TUN_METADATA0[], \ load:0x5566778899aabbcc->NXM_NX_TUN_METADATA1[],output:1" Then I dumped the data path flow rules, it would be like: set(tunnel(tun_id=0x3e8,dst=192.168.50.101,ttl=64, \ geneve({class=0xffff,type=0,len=4,0x11223344} \ {class=0xffff,type=0x1,len=8,0x5566778899aabbcc}),flags(df|key))),2 Even though I set the tunneling port's type to be VxLAN, the data plane flow will Be geneve, then the flows cannot be downloaded to the data plane. In the development, we must unbind the fields TUN_METADATAx from geneve tunneling Ports. So rename the these fields may also be required since the NSH header is not A tunneling encapsulation header. > > 3) Add "Ethernet + NSH" support for the OVS. Following the discussion at > > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > > we will introduce new match fields "encap_eth_dst/encap_eth_src" and > > related openflow actions"push_encap_eth/pop_encap_eth". Since we > > treat outer Ethernet header as encapsulation header, so NO VLAN and > > MPLS tags are supported for the outer header. That means packets with > > Format "Ethernet + VLAN[|MPLS] + NSH + Ethernet + IPv4[|IPv6] + > playload" > > Will be dropped by the OVS when parsing the packets. > > Examples for Ethernet NSH case: > > Please make a generalized push/pop Ethernet header rather than a specialized > one for this case. There are other use cases that involve this and they should be > consistent (i.e. connecting an L3 tunnel to an > L2 interface, MAC-in-MAC encapsulation, etc.) [JL] Since we introduced new fields encap_eth_dst/src, we know that the field differs from Origin eth_dst/src definitons. Then if we defines push_eth/pop_eth actions, but use the new field Definition encap_eth_dst/src as values to be pushed/popped, that would be confusing.
On Tue, Jun 21, 2016 at 6:50 PM, Li, Johnson <johnson.li@intel.com> wrote: >> On Mon, Jun 20, 2016 at 9:02 PM, Li, Johnson <johnson.li@intel.com> wrote: >> > 2) Add framework codes (or full implementation) for MD type 2 support of >> > the NSH header. According to the discussion at >> > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 >> > I will reuse the definitions for METADATA[0...63]. Since NSH is not one of >> > The tunneling encapsulation headers, renaming for the fields is required. >> > This might be a huge change for the current code base: >> >> Please see this discussion with you coworker from 6 months ago: >> http://openvswitch.org/pipermail/dev/2015-December/063306.html >> >> You guys already renamed the fields once. At this point, you need to live with >> what is there since we're not going to break compatibility with previous releases. >> > [JL] Yes, Mengke had renamed the command line and mapping table from xx-geneve-yy > To xx-tlv-yy. However, the flow fields are still named TUN_xxxx and bind to geneve > Tunneling ports. So there are some issues here. For example, I added one VxLAN port > With the following command: > # sudo ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 type=vxlan \ > options:dst_port=4790 options:remote_ip=192.168.50.101 \ > options:key=1000 options:exts=gpe > Then I tried to add some TLV map and then some flows: > #sudo ovs-ofctl add-tlv-map br-int "{class=0xffff,type=0,len=4}->tun_metadata0, \ > {class=0xffff,type=1,len=8}->tun_metadata1" > #sudo ovs-ofctl add-flow br-int "table=0, priority=260, in_port=LOCAL \ > actions=load:0x11223344->NXM_NX_TUN_METADATA0[], \ > load:0x5566778899aabbcc->NXM_NX_TUN_METADATA1[],output:1" > Then I dumped the data path flow rules, it would be like: > set(tunnel(tun_id=0x3e8,dst=192.168.50.101,ttl=64, \ > geneve({class=0xffff,type=0,len=4,0x11223344} \ > {class=0xffff,type=0x1,len=8,0x5566778899aabbcc}),flags(df|key))),2 > Even though I set the tunneling port's type to be VxLAN, the data plane flow will > Be geneve, then the flows cannot be downloaded to the data plane. Sure, this isn't surprising considering that Geneve is the only user of those fields in the current codebase. You'll need to find a way to generate the correct fields in this situation but that shouldn't involve any widespread renaming. > In the development, we must unbind the fields TUN_METADATAx from geneve tunneling > Ports. So rename the these fields may also be required since the NSH header is not > A tunneling encapsulation header. >> > 3) Add "Ethernet + NSH" support for the OVS. Following the discussion at >> > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 >> > we will introduce new match fields "encap_eth_dst/encap_eth_src" and >> > related openflow actions"push_encap_eth/pop_encap_eth". Since we >> > treat outer Ethernet header as encapsulation header, so NO VLAN and >> > MPLS tags are supported for the outer header. That means packets with >> > Format "Ethernet + VLAN[|MPLS] + NSH + Ethernet + IPv4[|IPv6] + >> playload" >> > Will be dropped by the OVS when parsing the packets. >> > Examples for Ethernet NSH case: >> >> Please make a generalized push/pop Ethernet header rather than a specialized >> one for this case. There are other use cases that involve this and they should be >> consistent (i.e. connecting an L3 tunnel to an >> L2 interface, MAC-in-MAC encapsulation, etc.) > [JL] Since we introduced new fields encap_eth_dst/src, we know that the field differs from > Origin eth_dst/src definitons. Then if we defines push_eth/pop_eth actions, but use the new field > Definition encap_eth_dst/src as values to be pushed/popped, that would be confusing. I'd like to have only one set of actions to do the same operation in different circumstances (and doesn't have arbitrary restrictions). There has been some work in OpenFlow as well as discussion on the OVS mailing list for some of these other scenarios which might be useful as a reference. Please investigate how you can have a clean solution that make sense in these different cases - this one seems too NSH-specific.
> > On Tue, Jun 21, 2016 at 6:50 PM, Li, Johnson <johnson.li@intel.com> wrote: > >> On Mon, Jun 20, 2016 at 9:02 PM, Li, Johnson <johnson.li@intel.com> > wrote: > > [JL] Yes, Mengke had renamed the command line and mapping table from > > xx-geneve-yy To xx-tlv-yy. However, the flow fields are still named > > TUN_xxxx and bind to geneve Tunneling ports. So there are some issues > > here. For example, I added one VxLAN port With the following command: > > # sudo ovs-vsctl add-port br-int vxlan0 -- set interface vxlan0 type=vxlan \ > > options:dst_port=4790 options:remote_ip=192.168.50.101 \ > > options:key=1000 options:exts=gpe Then I > > tried to add some TLV map and then some flows: > > #sudo ovs-ofctl add-tlv-map br-int "{class=0xffff,type=0,len=4}- > >tun_metadata0, \ > > {class=0xffff,type=1,len=8}->tun_metadata1" > > #sudo ovs-ofctl add-flow br-int "table=0, priority=260, in_port=LOCAL \ > > actions=load:0x11223344->NXM_NX_TUN_METADATA0[], \ > > load:0x5566778899aabbcc->NXM_NX_TUN_METADATA1[],output:1" > > Then I dumped the data path flow rules, it would be like: > > set(tunnel(tun_id=0x3e8,dst=192.168.50.101,ttl=64, \ > > geneve({class=0xffff,type=0,len=4,0x11223344} \ > > > > {class=0xffff,type=0x1,len=8,0x5566778899aabbcc}),flags(df|key))),2 > > Even though I set the tunneling port's type to be VxLAN, the data > > plane flow will Be geneve, then the flows cannot be downloaded to the > data plane. > > Sure, this isn't surprising considering that Geneve is the only user of those > fields in the current codebase. You'll need to find a way to generate the > correct fields in this situation but that shouldn't involve any widespread > renaming. > [JL]: I will try to make smallest change to make both geneve and NSH work. I will rename the definition which MUST be renamed. > > In the development, we must unbind the fields TUN_METADATAx from > > geneve tunneling Ports. So rename the these fields may also be > > required since the NSH header is not A tunneling encapsulation header. > http://comments.gmane.org/gmane.network.openvswitch.devel/53788 > >> Please make a generalized push/pop Ethernet header rather than a > >> specialized one for this case. There are other use cases that involve > >> this and they should be consistent (i.e. connecting an L3 tunnel to > >> an > >> L2 interface, MAC-in-MAC encapsulation, etc.) > > [JL] Since we introduced new fields encap_eth_dst/src, we know that > > the field differs from Origin eth_dst/src definitons. Then if we > > defines push_eth/pop_eth actions, but use the new field Definition > encap_eth_dst/src as values to be pushed/popped, that would be confusing. > > I'd like to have only one set of actions to do the same operation in different > circumstances (and doesn't have arbitrary restrictions). > There has been some work in OpenFlow as well as discussion on the OVS > mailing list for some of these other scenarios which might be useful as a > reference. Please investigate how you can have a clean solution that make > sense in these different cases - this one seems too NSH-specific. [JL]: Yes, there is another option is that we still use the field definition eth_dst/src For the key fields and we introduce push_eth/pop_eth flow actions. Once we use Flow command push_eth, we will add one eth header at the front of packet. The origin Ethernet header(if exists) will be treated as payload, while pop_eth will Always strip the outer Ethernet header. In this implementation, the origin packet's Ethernet header would not be parsed, so they could be used as match fields. Yi Yang would have some concern for this kind of implementation since he would Use the inner header as part of match fields in his user cases.
On Tue, Jun 21, 2016 at 7:40 PM, Li, Johnson <johnson.li@intel.com> wrote: >> On Tue, Jun 21, 2016 at 6:50 PM, Li, Johnson <johnson.li@intel.com> wrote: >> >> Please make a generalized push/pop Ethernet header rather than a >> >> specialized one for this case. There are other use cases that involve >> >> this and they should be consistent (i.e. connecting an L3 tunnel to >> >> an >> >> L2 interface, MAC-in-MAC encapsulation, etc.) >> > [JL] Since we introduced new fields encap_eth_dst/src, we know that >> > the field differs from Origin eth_dst/src definitons. Then if we >> > defines push_eth/pop_eth actions, but use the new field Definition >> encap_eth_dst/src as values to be pushed/popped, that would be confusing. >> >> I'd like to have only one set of actions to do the same operation in different >> circumstances (and doesn't have arbitrary restrictions). >> There has been some work in OpenFlow as well as discussion on the OVS >> mailing list for some of these other scenarios which might be useful as a >> reference. Please investigate how you can have a clean solution that make >> sense in these different cases - this one seems too NSH-specific. > [JL]: Yes, there is another option is that we still use the field definition eth_dst/src > For the key fields and we introduce push_eth/pop_eth flow actions. Once we use > Flow command push_eth, we will add one eth header at the front of packet. > The origin Ethernet header(if exists) will be treated as payload, while pop_eth will > Always strip the outer Ethernet header. In this implementation, the origin packet's > Ethernet header would not be parsed, so they could be used as match fields. > Yi Yang would have some concern for this kind of implementation since he would > Use the inner header as part of match fields in his user cases. Using a push/pop model seems better to me on the OpenFlow side as this would be consistent with VLAN/MPLS tags, which can also be stacked. Popping and then moving to a new table for an additional lookup could be used to access the inner headers if necessary. On the kernel side, it might be reasonable to parse multiple Ethernet headers in one pass to avoid a round of recirculation. This is possible since the kernel netlink interface handles nesting better and would be consistent with how MPLS is implemented and (eventually) VLAN. The reasonableness of that approach would depend on the extent to which it can avoid expanding the flow key in a way that would pessimize common use cases.
diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h index c955753..a1cca7e 100644 --- a/include/openvswitch/match.h +++ b/include/openvswitch/match.h @@ -86,6 +86,26 @@ void match_set_tun_gbp_id_masked(struct match *match, ovs_be16 gbp_id, ovs_be16 void match_set_tun_gbp_id(struct match *match, ovs_be16 gbp_id); void match_set_tun_gbp_flags_masked(struct match *match, uint8_t flags, uint8_t mask); void match_set_tun_gbp_flags(struct match *match, uint8_t flags); + +void match_set_nsp_masked(struct match *, ovs_be32 nsp, ovs_be32 mask); +void match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t mask); +void match_set_nsh_md_type_masked(struct match *match, uint8_t md_type, + uint8_t mask); +void match_set_nsh_next_proto_masked(struct match *match, uint8_t next_proto, + uint8_t mask); +void match_set_nshc1_masked(struct match *, ovs_be32 nshc1, ovs_be32 mask); +void match_set_nshc2_masked(struct match *, ovs_be32 nshc2, ovs_be32 mask); +void match_set_nshc3_masked(struct match *, ovs_be32 nshc3, ovs_be32 mask); +void match_set_nshc4_masked(struct match *, ovs_be32 nshc4, ovs_be32 mask); +void match_set_nsp(struct match *, ovs_be32 nsp); +void match_set_nsi(struct match *match, uint8_t nsi); +void match_set_nsh_md_type(struct match *match, uint8_t md_type); +void match_set_nsh_next_proto(struct match *match, uint8_t next_proto); +void match_set_nshc1(struct match *, ovs_be32 nshc1); +void match_set_nshc2(struct match *, ovs_be32 nshc2); +void match_set_nshc3(struct match *, ovs_be32 nshc3); +void match_set_nshc4(struct match *, ovs_be32 nshc4); + void match_set_in_port(struct match *, ofp_port_t ofp_port); void match_set_pkt_mark(struct match *, uint32_t pkt_mark); void match_set_pkt_mark_masked(struct match *, uint32_t pkt_mark, uint32_t mask); diff --git a/lib/match.c b/lib/match.c index f12c802..c0cc165 100644 --- a/lib/match.c +++ b/lib/match.c @@ -837,6 +837,112 @@ match_set_nd_target_masked(struct match *match, match->wc.masks.nd_target = *mask; } +void +match_set_nsp_masked(struct match *match, ovs_be32 nsp, ovs_be32 mask) +{ + match->wc.masks.nsh.nsp = mask; + match->flow.nsh.nsp = nsp & mask; +} + +void +match_set_nsi_masked(struct match *match, uint8_t nsi, uint8_t mask) +{ + match->wc.masks.nsh.nsi = mask; + match->flow.nsh.nsi = nsi & mask; +} + +void +match_set_nsh_md_type_masked(struct match *match, + uint8_t md_type, uint8_t mask) +{ + match->wc.masks.nsh.md_type = mask; + match->flow.nsh.md_type = md_type & mask; +} + +void +match_set_nsh_next_proto_masked(struct match *match, + uint8_t next_proto, uint8_t mask) +{ + match->wc.masks.nsh.next_proto = mask; + match->flow.nsh.next_proto = next_proto & mask; +} + +void +match_set_nshc1_masked(struct match *match, ovs_be32 nshc1, ovs_be32 mask) +{ + match->wc.masks.nsh.nshc1 = mask; + match->flow.nsh.nshc1 = nshc1 & mask; +} + +void +match_set_nshc2_masked(struct match *match, ovs_be32 nshc2, ovs_be32 mask) +{ + match->wc.masks.nsh.nshc2 = mask; + match->flow.nsh.nshc2 = nshc2 & mask; +} + +void +match_set_nshc3_masked(struct match *match, ovs_be32 nshc3, ovs_be32 mask) +{ + match->wc.masks.nsh.nshc3 = mask; + match->flow.nsh.nshc3 = nshc3 & mask; +} + +void +match_set_nshc4_masked(struct match *match, ovs_be32 nshc4, ovs_be32 mask) +{ + match->wc.masks.nsh.nshc4 = mask; + match->flow.nsh.nshc4 = nshc4 & mask; +} + +void +match_set_nsp(struct match *match, ovs_be32 nsp) +{ + match_set_nsp_masked(match, nsp, OVS_BE32_MAX); +} + +void +match_set_nsi(struct match *match, uint8_t nsi) +{ + match_set_nsi_masked(match, nsi, UINT8_MAX); +} + +void +match_set_nsh_md_type(struct match *match, uint8_t md_type) +{ + match_set_nsh_md_type_masked(match, md_type, UINT8_MAX); +} + +void +match_set_nsh_next_proto(struct match *match, uint8_t next_proto) +{ + match_set_nsh_next_proto_masked(match, next_proto, UINT8_MAX); +} + +void +match_set_nshc1(struct match *match, ovs_be32 nshc1) +{ + match_set_nshc1_masked(match, nshc1, OVS_BE32_MAX); +} + +void +match_set_nshc2(struct match *match, ovs_be32 nshc2) +{ + match_set_nshc2_masked(match, nshc2, OVS_BE32_MAX); +} + +void +match_set_nshc3(struct match *match, ovs_be32 nshc3) +{ + match_set_nshc3_masked(match, nshc3, OVS_BE32_MAX); +} + +void +match_set_nshc4(struct match *match, ovs_be32 nshc4) +{ + match_set_nshc4_masked(match, nshc4, OVS_BE32_MAX); +} + /* Returns true if 'a' and 'b' wildcard the same fields and have the same * values for fixed fields, otherwise false. */ bool
Signed-off-by: Johnson Li <johnson.li@intel.com>