Hi Martin, See my comments below...
Cheers, Eelco On 30 Aug 2021, at 14:40, Martin Varghese wrote: > From: Martin Varghese <[email protected]> > > The encap & decap actions are extended to support MPLS packet type. > Encap & decap actions adds and removes MPLS header at start of the > packet. > > The existing PUSH MPLS & POP MPLS actions inserts & removes MPLS > header between ethernet header and the IP header. Though this behaviour > is fine for L3 VPN where an IP packet is encapsulated inside a MPLS > tunnel, it does not suffice the L2 VPN requirements. In L2 VPN the > ethernet packets must be encapsulated inside MPLS tunnel. > > In this change the encap & decap actions are extended to support MPLS > packet type. The encap & decap adds and removes MPLS header at the > start of packet as depicted below. > > Encapsulation: > > Actions - encap(mpls(ether_type=0x8847)),encap(ethernet) > > Incoming packet -> | ETH | IP | Payload | > > 1 Actions - encap(mpls(ether_type=0x8847)) [Datapath action - > ADD_MPLS:0x8847] > > Outgoing packet -> | MPLS | ETH | Payload| > > 2 Actions - encap(ethernet) [ Datapath action - push_eth ] > > Outgoing packet -> | ETH | MPLS | ETH | Payload| > > Decapsulation: > > Incoming packet -> | ETH | MPLS | ETH | IP | Payload | > > Actions - decap(),decap(packet_type(ns=0,type=0) > > 1 Actions - decap() [Datapath action - pop_eth) > > Outgoing packet -> | MPLS | ETH | IP | Payload| > > 2 Actions - decap(packet_type(ns=0,type=0) [Datapath action - POP_MPLS:0x6558] > > Outgoing packet -> | ETH | IP | Payload| > > Signed-off-by: Martin Varghese <[email protected]> > --- > Changes in v2: > - Fixed the compilation error reported by bot. > > Changes in v3: > - Adapted the changes to allign with the kernel implementaion > > Changes in v4: > - Fixed the compilation error reported by bot. > - Added SLOW_ACTION support for no datapath support. > > Changes in v5: > > - Code styling fixed. > - Given reference for packet_type field in documentation. > - Modified code to do recirc only for the last label. > - Cleaed l2,l3,l4 fields with encap. > - Fixed existing encap & decap tests to do set eth to outer header > - Added tests to verify Encap & Decap with legacy Push & Pop. > - Added xlate tests. > - Added seperate tests to inspect packet after encap & decap. > - Added tests for multiple mpls test cases. > > NEWS | 2 +- > .../linux/compat/include/linux/openvswitch.h | 33 ++- > include/openvswitch/ofp-ed-props.h | 18 ++ > lib/dpif-netdev.c | 1 + > lib/dpif.c | 1 + > lib/odp-execute.c | 12 + > lib/odp-util.c | 50 +++- > lib/ofp-actions.c | 5 + > lib/ofp-ed-props.c | 91 +++++++ > lib/ovs-actions.xml | 32 ++- > lib/packets.c | 47 +++- > lib/packets.h | 2 + > ofproto/ofproto-dpif-ipfix.c | 1 + > ofproto/ofproto-dpif-sflow.c | 1 + > ofproto/ofproto-dpif-xlate.c | 74 ++++++ > ofproto/ofproto-dpif.c | 39 +++ > ofproto/ofproto-dpif.h | 4 +- > tests/mpls-xlate.at | 47 ++++ > tests/system-traffic.at | 227 ++++++++++++++++++ > 19 files changed, 664 insertions(+), 23 deletions(-) > > diff --git a/NEWS b/NEWS > index 1f2adf718..8afc62534 100644 > --- a/NEWS > +++ b/NEWS > @@ -209,7 +209,7 @@ v2.14.0 - 17 Aug 2020 > - GTP-U Tunnel Protocol > * Add two new fields: tun_gtpu_flags, tun_gtpu_msgtype. > * Only support for userspace datapath. > - > + - Encap & Decap action support for MPLS packet type. > > v2.13.0 - 14 Feb 2020 > --------------------- > diff --git a/datapath/linux/compat/include/linux/openvswitch.h > b/datapath/linux/compat/include/linux/openvswitch.h > index f0595eeba..88a653771 100644 > --- a/datapath/linux/compat/include/linux/openvswitch.h > +++ b/datapath/linux/compat/include/linux/openvswitch.h > @@ -817,8 +817,32 @@ struct ovs_action_push_tnl { > }; > #endif > > -/** > - * enum ovs_ct_attr - Attributes for %OVS_ACTION_ATTR_CT action. > +/* struct ovs_action_add_mpls - %OVS_ACTION_ATTR_ADD_MPLS action > + * argument. > + * @mpls_lse: MPLS label stack entry to push. > + * @mpls_ethertype: Ethertype to set in the encapsulating ethernet frame. > + * @tun_flags: MPLS tunnel attributes. > + * > + * The only values @mpls_ethertype should ever be given are %ETH_P_MPLS_UC > and > + * %ETH_P_MPLS_MC, indicating MPLS unicast or multicast. Other are rejected. > + */ > +struct ovs_action_add_mpls { > + __be32 mpls_lse; > + __be16 mpls_ethertype; /* Either %ETH_P_MPLS_UC or %ETH_P_MPLS_MC */ > + __u16 tun_flags; > +}; > + > +#define OVS_MPLS_L3_TUNNEL_FLAG_MASK (1 << 0) /* Flag to specify the place > of > + * insertion of MPLS header. > + * When false, the MPLS header > + * will be inserted at the start > + * of the packet. > + * When true, the MPLS header > + * will be inserted at the start > + * of the l3 header. > + */ > + > +/* enum ovs_ct_attr - Attributes for %OVS_ACTION_ATTR_CT action. > * @OVS_CT_ATTR_COMMIT: If present, commits the connection to the conntrack > * table. This allows future packets for the same connection to be identified > * as 'established' or 'related'. The flow key for the current packet will > @@ -1008,6 +1032,10 @@ struct check_pkt_len_arg { > * @OVS_ACTION_ATTR_CHECK_PKT_LEN: Check the packet length and execute a set > * of actions if greater than the specified packet length, else execute > * another set of actions. > + * @OVS_ACTION_ATTR_ADD_MPLS: Push a new MPLS label stack entry at the > + * start of the packet or at the start of the l3 header depending on the > value > + * of l3 tunnel flag in the tun_flags field of OVS_ACTION_ATTR_ADD_MPLS > + * argument. > * @OVS_ACTION_ATTR_DROP: Explicit drop action. > */ > > @@ -1037,6 +1065,7 @@ enum ovs_action_attr { > OVS_ACTION_ATTR_METER, /* u32 meter number. */ > OVS_ACTION_ATTR_CLONE, /* Nested OVS_CLONE_ATTR_*. */ > OVS_ACTION_ATTR_CHECK_PKT_LEN, /* Nested OVS_CHECK_PKT_LEN_ATTR_*. */ > + OVS_ACTION_ATTR_ADD_MPLS, /* struct ovs_action_add_mpls. */ > > #ifndef __KERNEL__ > OVS_ACTION_ATTR_TUNNEL_PUSH, /* struct ovs_action_push_tnl*/ > diff --git a/include/openvswitch/ofp-ed-props.h > b/include/openvswitch/ofp-ed-props.h > index 306c6fe73..c85f3c283 100644 > --- a/include/openvswitch/ofp-ed-props.h > +++ b/include/openvswitch/ofp-ed-props.h > @@ -46,6 +46,11 @@ enum ofp_ed_nsh_prop_type { > OFPPPT_PROP_NSH_TLV = 2, /* property TLV in NSH */ > }; > > +enum ofp_ed_mpls_prop_type { > + OFPPPT_PROP_MPLS_NONE = 0, /* unused */ Can you align the above comment to the one below? > + OFPPPT_PROP_MPLS_ETHERTYPE = 1, /* MPLS Ethertype */ > +}; > + > /* > * External representation of encap/decap properties. > * These must be padded to a multiple of 8 bytes. > @@ -72,6 +77,13 @@ struct ofp_ed_prop_nsh_tlv { > uint8_t data[0]; > }; > > +struct ofp_ed_prop_mpls_ethertype { > + struct ofp_ed_prop_header header; > + uint16_t ether_type; /* MPLS ethertype .*/ Can you align the above comment to the one below? > + uint8_t pad[2]; /* Padding to 8 bytes. */ > +}; > + > + > /* > * Internal representation of encap/decap properties > */ > @@ -96,6 +108,12 @@ struct ofpact_ed_prop_nsh_tlv { > /* tlv_len octets of metadata value, padded to a multiple of 8 bytes. */ > uint8_t data[0]; > }; > + > +struct ofpact_ed_prop_mpls_ethertype { > + struct ofpact_ed_prop header; > + uint16_t ether_type; /* MPLS ethertype .*/ > + uint8_t pad[2]; /* Padding to 8 bytes. */ > +}; > enum ofperr decode_ed_prop(const struct ofp_ed_prop_header **ofp_prop, > struct ofpbuf *out, size_t *remaining); > enum ofperr encode_ed_prop(const struct ofpact_ed_prop **prop, > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c > index b3e57bb95..1d3287c32 100644 > --- a/lib/dpif-netdev.c > +++ b/lib/dpif-netdev.c > @@ -8293,6 +8293,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch > *packets_, > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > case OVS_ACTION_ATTR_DROP: > + case OVS_ACTION_ATTR_ADD_MPLS: > case __OVS_ACTION_ATTR_MAX: > OVS_NOT_REACHED(); > } > diff --git a/lib/dpif.c b/lib/dpif.c > index 8c4aed47b..99aba3cae 100644 > --- a/lib/dpif.c > +++ b/lib/dpif.c > @@ -1274,6 +1274,7 @@ dpif_execute_helper_cb(void *aux_, struct > dp_packet_batch *packets_, > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > case OVS_ACTION_ATTR_DROP: > + case OVS_ACTION_ATTR_ADD_MPLS: > case __OVS_ACTION_ATTR_MAX: > OVS_NOT_REACHED(); > } > diff --git a/lib/odp-execute.c b/lib/odp-execute.c > index 6eeda2a61..2f4cdd92c 100644 > --- a/lib/odp-execute.c > +++ b/lib/odp-execute.c > @@ -819,6 +819,7 @@ requires_datapath_assistance(const struct nlattr *a) > case OVS_ACTION_ATTR_POP_NSH: > case OVS_ACTION_ATTR_CT_CLEAR: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > + case OVS_ACTION_ATTR_ADD_MPLS: > case OVS_ACTION_ATTR_DROP: > return false; > > @@ -1061,6 +1062,17 @@ odp_execute_actions(void *dp, struct dp_packet_batch > *batch, bool steal, > } > break; > > + case OVS_ACTION_ATTR_ADD_MPLS: { > + const struct ovs_action_add_mpls *mpls = nl_attr_get(a); > + bool l3_flag = mpls->tun_flags & OVS_MPLS_L3_TUNNEL_FLAG_MASK; > + > + DP_PACKET_BATCH_FOR_EACH (i, packet, batch) { > + add_mpls(packet, mpls->mpls_ethertype, mpls->mpls_lse, > + l3_flag); > + } > + break; > + } > + > case OVS_ACTION_ATTR_DROP:{ > const enum xlate_error *drop_reason = nl_attr_get(a); > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 7729a9060..5f5dee35b 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -142,6 +142,7 @@ odp_action_len(uint16_t type) > case OVS_ACTION_ATTR_PUSH_NSH: return ATTR_LEN_VARIABLE; > case OVS_ACTION_ATTR_POP_NSH: return 0; > case OVS_ACTION_ATTR_CHECK_PKT_LEN: return ATTR_LEN_VARIABLE; > + case OVS_ACTION_ATTR_ADD_MPLS: return sizeof(struct ovs_action_add_mpls); > case OVS_ACTION_ATTR_DROP: return sizeof(uint32_t); > > case OVS_ACTION_ATTR_UNSPEC: > @@ -1254,6 +1255,14 @@ format_odp_action(struct ds *ds, const struct nlattr > *a, > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > format_odp_check_pkt_len_action(ds, a, portno_names); > break; > + case OVS_ACTION_ATTR_ADD_MPLS: { > + const struct ovs_action_push_mpls *mpls = nl_attr_get(a); > + ds_put_cstr(ds, "add_mpls("); > + format_mpls_lse(ds, mpls->mpls_lse); > + ds_put_format(ds, ",eth_type=0x%"PRIx16")", > + ntohs(mpls->mpls_ethertype)); Can we add a test case for this new dp action in odp.at:AT_SETUP([OVS datapath actions parsing and formatting - valid forms])? > + break; > + } > case OVS_ACTION_ATTR_DROP: > ds_put_cstr(ds, "drop"); > break; > @@ -7890,7 +7899,7 @@ commit_vlan_action(const struct flow* flow, struct flow > *base, > /* Wildcarding already done at action translation time. */ > static void > commit_mpls_action(const struct flow *flow, struct flow *base, > - struct ofpbuf *odp_actions) > + struct ofpbuf *odp_actions, bool pending_encap) > { > int base_n = flow_count_mpls_labels(base, NULL); > int flow_n = flow_count_mpls_labels(flow, NULL); > @@ -7938,18 +7947,29 @@ commit_mpls_action(const struct flow *flow, struct > flow *base, > /* If, after the above popping and setting, there are more LSEs in flow > * than base then some LSEs need to be pushed. */ > while (base_n < flow_n) { > - struct ovs_action_push_mpls *mpls; > > - mpls = nl_msg_put_unspec_zero(odp_actions, > - OVS_ACTION_ATTR_PUSH_MPLS, > - sizeof *mpls); > - mpls->mpls_ethertype = flow->dl_type; > - mpls->mpls_lse = flow->mpls_lse[flow_n - base_n - 1]; > + if (pending_encap) { > + struct ovs_action_add_mpls *mpls; > + > + mpls = nl_msg_put_unspec_zero(odp_actions, > + OVS_ACTION_ATTR_ADD_MPLS, > + sizeof *mpls); > + mpls->mpls_ethertype = flow->dl_type; > + mpls->mpls_lse = flow->mpls_lse[flow_n - base_n - 1]; > + } else { > + struct ovs_action_push_mpls *mpls; > + > + mpls = nl_msg_put_unspec_zero(odp_actions, > + OVS_ACTION_ATTR_PUSH_MPLS, > + sizeof *mpls); > + mpls->mpls_ethertype = flow->dl_type; > + mpls->mpls_lse = flow->mpls_lse[flow_n - base_n - 1]; > + } > /* Update base flow's MPLS stack, but do not clear L3. We need the > L3 > * headers if the flow is restored later due to returning from a > patch > * port or group bucket. */ > - flow_push_mpls(base, base_n, mpls->mpls_ethertype, NULL, false); > - flow_set_mpls_lse(base, 0, mpls->mpls_lse); > + flow_push_mpls(base, base_n, flow->dl_type, NULL, false); > + flow_set_mpls_lse(base, 0, flow->mpls_lse[flow_n - base_n - 1]); > base_n++; > } > } > @@ -8600,6 +8620,10 @@ commit_encap_decap_action(const struct flow *flow, > memcpy(&base_flow->dl_dst, &flow->dl_dst, > sizeof(*flow) - offsetof(struct flow, dl_dst)); > break; > + case PT_MPLS: > + commit_mpls_action(flow, base_flow, odp_actions, > + pending_encap); > + break; > default: > /* Only the above protocols are supported for encap. > * The check is done at action translation. */ > @@ -8622,6 +8646,10 @@ commit_encap_decap_action(const struct flow *flow, > /* pop_nsh. */ > odp_put_pop_nsh_action(odp_actions); > break; > + case PT_MPLS: > + commit_mpls_action(flow, base_flow, odp_actions, > + pending_encap); > + break; > default: > /* Checks are done during translation. */ > OVS_NOT_REACHED(); > @@ -8667,7 +8695,7 @@ commit_odp_actions(const struct flow *flow, struct flow > *base, > /* Make packet a non-MPLS packet before committing L3/4 actions, > * which would otherwise do nothing. */ > if (eth_type_mpls(base->dl_type) && !eth_type_mpls(flow->dl_type)) { > - commit_mpls_action(flow, base, odp_actions); > + commit_mpls_action(flow, base, odp_actions, false); > mpls_done = true; > } > commit_set_nsh_action(flow, base, odp_actions, wc, use_masked); > @@ -8675,7 +8703,7 @@ commit_odp_actions(const struct flow *flow, struct flow > *base, > commit_set_port_action(flow, base, odp_actions, wc, use_masked); > slow2 = commit_set_icmp_action(flow, base, odp_actions, wc); > if (!mpls_done) { > - commit_mpls_action(flow, base, odp_actions); > + commit_mpls_action(flow, base, odp_actions, false); > } > commit_vlan_action(flow, base, odp_actions, wc); > commit_set_priority_action(flow, base, odp_actions, wc, use_masked); > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c > index ecf914eac..8b74386b1 100644 > --- a/lib/ofp-actions.c > +++ b/lib/ofp-actions.c > @@ -4468,6 +4468,7 @@ decode_NXAST_RAW_ENCAP(const struct nx_action_encap > *nae, > switch (ntohl(nae->new_pkt_type)) { > case PT_ETH: > case PT_NSH: > + case PT_MPLS: > /* Add supported encap header types here. */ > break; > default: > @@ -4519,6 +4520,8 @@ parse_encap_header(const char *hdr, ovs_be32 > *packet_type) > *packet_type = htonl(PT_ETH); > } else if (strcmp(hdr, "nsh") == 0) { > *packet_type = htonl(PT_NSH); > + } else if (strcmp(hdr, "mpls") == 0) { > + *packet_type = htonl(PT_MPLS); > } else { > return false; > } > @@ -4600,6 +4603,8 @@ format_encap_pkt_type(const ovs_be32 pkt_type) > return "ethernet"; > case PT_NSH: > return "nsh"; > + case PT_MPLS: > + return "mpls"; > default: > return "UNKNOWN"; > } > diff --git a/lib/ofp-ed-props.c b/lib/ofp-ed-props.c > index 02a9235d5..fc261e4c6 100644 > --- a/lib/ofp-ed-props.c > +++ b/lib/ofp-ed-props.c > @@ -79,6 +79,27 @@ decode_ed_prop(const struct ofp_ed_prop_header **ofp_prop, > } > break; > } > + case OFPPPC_MPLS: { > + switch (prop_type) { "switch" should be move one space to the right. > + case OFPPPT_PROP_MPLS_ETHERTYPE: { > + struct ofp_ed_prop_mpls_ethertype *opnmt = Guess you copied this from above, but based on the naming I guess opnmt should be opmet (or even opme) > + ALIGNED_CAST(struct ofp_ed_prop_mpls_ethertype *, *ofp_prop); > + if (len > sizeof(*opnmt) || len > *remaining) { > + return OFPERR_NXBAC_BAD_ED_PROP; > + } > + struct ofpact_ed_prop_mpls_ethertype *pnmt = Same here guess it should be '*pmet = ' > + ofpbuf_put_uninit(out, sizeof(*pnmt)); > + pnmt->header.prop_class = prop_class; > + pnmt->header.type = prop_type; > + pnmt->header.len = len; > + pnmt->ether_type = opnmt->ether_type; > + break; > + } > + default: > + return OFPERR_NXBAC_UNKNOWN_ED_PROP; > + } > + break; > + } > default: > return OFPERR_NXBAC_UNKNOWN_ED_PROP; > } > @@ -134,6 +155,27 @@ encode_ed_prop(const struct ofpact_ed_prop **prop, > } > break; > } > + case OFPPPC_MPLS: { > + switch ((*prop)->type) { Indentation of switch (and lines below) > + case OFPPPT_PROP_MPLS_ETHERTYPE: { > + struct ofpact_ed_prop_mpls_ethertype *pnmt = See naming above pme(t) > + ALIGNED_CAST(struct ofpact_ed_prop_mpls_ethertype *, *prop); > + struct ofp_ed_prop_mpls_ethertype *opnmt = > + ofpbuf_put_uninit(out, sizeof(*opnmt)); See naming above opme(t) > + opnmt->header.prop_class = htons((*prop)->prop_class); > + opnmt->header.type = (*prop)->type; > + opnmt->header.len = > + offsetof(struct ofpact_ed_prop_mpls_ethertype, pad); > + opnmt->ether_type = pnmt->ether_type; > + prop_len = sizeof(*pnmt); > + break; > + Remove empty line > + } > + default: > + return OFPERR_OFPBAC_BAD_ARGUMENT; > + } > + break; > + } > default: > return OFPERR_OFPBAC_BAD_ARGUMENT; > } > @@ -181,6 +223,13 @@ parse_ed_prop_type(uint16_t prop_class, > } else { > return false; > } > + case OFPPPC_MPLS: > + if (!strcmp(str, "ether_type")) { > + *type = OFPPPT_PROP_MPLS_ETHERTYPE; > + return true; > + } else { > + return false; > + } > default: > return false; > } > @@ -259,6 +308,28 @@ parse_ed_prop_value(uint16_t prop_class, uint8_t > prop_type OVS_UNUSED, > OVS_NOT_REACHED(); > } > break; > + case OFPPPC_MPLS: > + switch (prop_type) { > + case OFPPPT_PROP_MPLS_ETHERTYPE: { > + uint16_t ethertype; > + error = str_to_u16(value, "ether_type", ðertype); > + if (error != NULL) { > + return error; > + } > + struct ofpact_ed_prop_mpls_ethertype *pnmt = > + ofpbuf_put_uninit(out, sizeof(*pnmt)); See naming above pme(t) > + pnmt->header.prop_class = prop_class; > + pnmt->header.type = prop_type; > + pnmt->header.len = > + offsetof(struct ofpact_ed_prop_mpls_ethertype, pad); > + pnmt->ether_type = ethertype; > + > + break; > + } > + default: Default should be same as other types: /* Unsupported property types rejected before. */ OVS_NOT_REACHED(); > + break; > + } > + break; > default: > /* Unsupported property classes rejected before. */ > OVS_NOT_REACHED(); > @@ -300,6 +371,14 @@ format_ed_prop_type(const struct ofpact_ed_prop *prop) > OVS_NOT_REACHED(); > } > break; > + case OFPPPC_MPLS: Indentation of the below is off by one space. > + switch (prop->type) { > + case OFPPPT_PROP_MPLS_ETHERTYPE: > + return "ether_type"; > + default: > + OVS_NOT_REACHED(); > + } > + break; > default: > OVS_NOT_REACHED(); > } > @@ -332,6 +411,18 @@ format_ed_prop(struct ds *s OVS_UNUSED, > default: > OVS_NOT_REACHED(); > } > + case OFPPPC_MPLS: Check alignment. > + switch (prop->type) { > + case OFPPPT_PROP_MPLS_ETHERTYPE: { > + struct ofpact_ed_prop_mpls_ethertype *pnmt = > + ALIGNED_CAST(struct ofpact_ed_prop_mpls_ethertype *, prop); > + ds_put_format(s, "%s=%d", format_ed_prop_type(prop), > + pnmt->ether_type); > + return; > + } > + default: > + OVS_NOT_REACHED(); > + } > default: > OVS_NOT_REACHED(); > } > diff --git a/lib/ovs-actions.xml b/lib/ovs-actions.xml > index c94b5f3b3..a0cdab85c 100644 > --- a/lib/ovs-actions.xml > +++ b/lib/ovs-actions.xml The xml based documentation has been converted to Documentation/ref/ovs-actions.7.rst, so you need to update it there. > @@ -265,13 +265,13 @@ > </p> > > <p> > - When a <code>decap</code> action decapsulates a packet, Open vSwitch > - raises this error if it does not support the type of inner packet. > - <code>decap</code> of an Ethernet header raises this error if a VLAN > - header is present, <code>decap</code> of a NSH packet raises this > error > - if the NSH inner packet is not Ethernet, IPv4, IPv6, or NSH, and > - <code>decap</code> of other types of packets is unsupported and also > - raises this error. > + The <code>decap</code> action is supported only for packet types > + ethernet, NSH and MPLS. Openvswitch raises this error for other > + packet types. When a <code>decap</code> action decapsulates a packet, > + Open vSwitch raises this error if it does not support the type of > inner > + packet. <code>decap</code> of an Ethernet header raises this error > if a > + VLAN header is present, <code>decap</code> of a NSH packet raises > this > + error if the NSH inner packet is not Ethernet, IPv4, IPv6, or NSH. > </p> > > <p> > @@ -1396,6 +1396,8 @@ for <var>i</var> in [1,<var>n_members</var>]: > <h2>The <code>encap</code> action</h2> > > <syntax><code>encap(nsh(</code>[<code>md_type=<var>md_type</var></code>]<code>, > > </code>[<code>tlv(<var>class</var>,<var>type</var>,<var>value</var>)</code>]...<code>))</code></syntax> > <syntax><code>encap(ethernet)</code></syntax> > + <syntax><code>encap(mpls(ether_type=<var>ether_type</var>))</code> > + </syntax> > > <p> > The <code>encap</code> action encapsulates a packet with a specified > @@ -1434,6 +1436,12 @@ for <var>i</var> in [1,<var>n_members</var>]: > source and destination are initially zeroed. > </p> > > + <p> > + The <code>encap(mpls(ethertype=....))</code> variant encapsulates an > + ethernet or L3 packet with a MPLS header. The <var>ethertype</var> > + could be MPLS unicast (0x8847) or multicast (0x8848) ethertypes. > + </p> > + > <conformance> > This action is an Open vSwitch extension to OpenFlow 1.3 and later, > introduced in Open vSwitch 2.8. > @@ -1443,6 +1451,9 @@ for <var>i</var> in [1,<var>n_members</var>]: > <action name="DECAP"> > <h2>The <code>decap</code> action</h2> > <syntax><code>decap</code></syntax> > + <syntax><code>decap(packet_type(ns=<var>name_space</var>, > + type=<var>ethertype</var>))</code></syntax> for decapsulating MPLS > + packets. > > <p> > Removes an outermost encapsulation from the packet: > @@ -1463,6 +1474,13 @@ for <var>i</var> in [1,<var>n_members</var>]: > packet type errors. > </li> > > + <li> > + Otherwise, if the packet is a MPLS packet, removes the MPLS header > + and classifies the inner packet as mentioned in the packet type > + argument of the decap. The packet_type field specifies the type of > + the packet in the format specified in OpenFlow 1.5. Can we explain more what the ns and ethertype values mean? Or maybe be more specific and add reference to OpenFlow 1.5 chapter "7.2.3.11 Packet Type Match Field" > + </li> > + > <li> > Otherwise, raises an unsupported packet type error. > </li> > diff --git a/lib/packets.c b/lib/packets.c > index 4a7643c5d..66fefdaba 100644 > --- a/lib/packets.c > +++ b/lib/packets.c > @@ -418,6 +418,38 @@ push_mpls(struct dp_packet *packet, ovs_be16 ethtype, > ovs_be32 lse) > pkt_metadata_init_conn(&packet->md); > } > > +void > +add_mpls(struct dp_packet *packet, ovs_be16 ethtype, ovs_be32 lse, bool l3) Maybe rename l3 to l3_encap to be more explicit? > +{ > + char * header; Remove space after * > + > + if (!eth_type_mpls(ethtype)) { > + return; > + } > + > + if (!l3) { > + header = dp_packet_push_uninit(packet, MPLS_HLEN); Remove extra space after = Also, does it maybe make more sense to do a direct assignment here instead of memcpy, or do we rely on the compiler to do this for us (also below)? ovs_be32 *header = dp_packet_push_uninit(packet, MPLS_HLEN); *header = lse; > + memcpy(header, &lse, sizeof lse); > + packet->l2_5_ofs = 0; > + packet->packet_type = htonl(PT_MPLS); > + } else { > + size_t len; > + > + if (!is_mpls(packet)) { > + /* Set MPLS label stack offset. */ > + packet->l2_5_ofs = packet->l3_ofs; > + } > + set_ethertype(packet, ethtype); > + > + /* Push new MPLS shim header onto packet. */ > + len = packet->l2_5_ofs; > + header = dp_packet_resize_l2_5(packet, MPLS_HLEN); > + memmove(header, header + MPLS_HLEN, len); > + memcpy(header + len, &lse, sizeof lse); > + } > + pkt_metadata_init_conn(&packet->md); > +} > + > /* If 'packet' is an MPLS packet, removes its outermost MPLS label stack > entry. > * If the label that was removed was the only MPLS label, changes 'packet''s > * Ethertype to 'ethtype' (which ordinarily should not be an MPLS > @@ -426,10 +458,14 @@ void > pop_mpls(struct dp_packet *packet, ovs_be16 ethtype) > { > if (is_mpls(packet)) { > + Remove introduced new line > struct mpls_hdr *mh = dp_packet_l2_5(packet); > size_t len = packet->l2_5_ofs; > > - set_ethertype(packet, ethtype); > + if (ethtype != htons(ETH_TYPE_TEB)) { It looks like we are using ETH_TYPE_TEB as some magic marker, what happens if a packet really has ETH_TYPE_TEB? Can you explain the rational behind this? > + set_ethertype(packet, ethtype); Indentation is one to deep. > + } > + > if (get_16aligned_be32(&mh->mpls_lse) & htonl(MPLS_BOS_MASK)) { > dp_packet_set_l2_5(packet, NULL); > } > @@ -440,6 +476,15 @@ pop_mpls(struct dp_packet *packet, ovs_be16 ethtype) > /* Invalidate offload flags as they are not valid after > * decapsulation of MPLS header. */ > dp_packet_reset_offload(packet); Can we group the above and below ethtype checks/mods together and add some comments, so it's clear why there is a difference, i.e. L2 vs L3? > + > + if (!len) { > + if (ethtype == htons(ETH_TYPE_TEB)) { > + packet->packet_type = htonl(PT_ETH); > + } else { > + packet->packet_type = PACKET_TYPE_BE(OFPHTN_ETHERTYPE, > + ntohs(ethtype)); > + } > + } > } > } > > diff --git a/lib/packets.h b/lib/packets.h > index 515bb59b1..8f72af328 100644 > --- a/lib/packets.h > +++ b/lib/packets.h > @@ -356,6 +356,8 @@ void set_mpls_lse_label(ovs_be32 *lse, ovs_be32 label); > void set_mpls_lse_bos(ovs_be32 *lse, uint8_t bos); > ovs_be32 set_mpls_lse_values(uint8_t ttl, uint8_t tc, uint8_t bos, > ovs_be32 label); > +void add_mpls(struct dp_packet *packet, ovs_be16 ethtype, ovs_be32 lse, > + bool l3_flag); l3_flag, to l3_encap (see above). > > /* Example: > * > diff --git a/ofproto/ofproto-dpif-ipfix.c b/ofproto/ofproto-dpif-ipfix.c > index 796eb6f88..9280e008e 100644 > --- a/ofproto/ofproto-dpif-ipfix.c > +++ b/ofproto/ofproto-dpif-ipfix.c > @@ -3018,6 +3018,7 @@ dpif_ipfix_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_DROP: > + case OVS_ACTION_ATTR_ADD_MPLS: > case __OVS_ACTION_ATTR_MAX: > default: > break; > diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c > index 864c136b5..30e7caf54 100644 > --- a/ofproto/ofproto-dpif-sflow.c > +++ b/ofproto/ofproto-dpif-sflow.c > @@ -1226,6 +1226,7 @@ dpif_sflow_read_actions(const struct flow *flow, > case OVS_ACTION_ATTR_UNSPEC: > case OVS_ACTION_ATTR_CHECK_PKT_LEN: > case OVS_ACTION_ATTR_DROP: > + case OVS_ACTION_ATTR_ADD_MPLS: > case __OVS_ACTION_ATTR_MAX: > default: > break; > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 8723cb4e8..831c71275 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -6403,6 +6403,50 @@ rewrite_flow_encap_ethernet(struct xlate_ctx *ctx, > ctx->error = XLATE_UNSUPPORTED_PACKET_TYPE; > } > } > +static void > +rewrite_flow_encap_mpls(struct xlate_ctx *ctx, > + const struct ofpact_encap *encap, > + struct flow *flow, > + struct flow_wildcards *wc) > +{ > + int n; > + uint32_t i; > + uint16_t ether_type; > + const char *ptr = (char *) encap->props; > + > + for (i = 0; i < encap->n_props; i++) { "for" is not aligned correctly, also look at alignment below. > + struct ofpact_ed_prop *prop_ptr = > + ALIGNED_CAST(struct ofpact_ed_prop *, ptr); > + if (prop_ptr->prop_class == OFPPPC_MPLS) { > + switch (prop_ptr->type) { > + case OFPPPT_PROP_MPLS_ETHERTYPE: { > + struct ofpact_ed_prop_mpls_ethertype *prop_ether_type = > + ALIGNED_CAST(struct ofpact_ed_prop_mpls_ethertype *, > + prop_ptr); > + ether_type = prop_ether_type->ether_type; > + break; > + } > + } > + } > + } > + > + wc->masks.packet_type = OVS_BE32_MAX; > + if (flow->packet_type != htonl(PT_MPLS)) { > + memset(&ctx->wc->masks.mpls_lse, 0x0, > + sizeof *wc->masks.mpls_lse * FLOW_MAX_MPLS_LABELS); > + memset(&flow->mpls_lse, 0x0, sizeof *flow->mpls_lse * > + FLOW_MAX_MPLS_LABELS); > + memset(&ctx->base_flow.mpls_lse, 0x0, sizeof > *ctx->base_flow.mpls_lse * > + FLOW_MAX_MPLS_LABELS); Do we really need to memset these? Asking as normally these get cleared at start? If so it might be worth adding a comment why they need to be cleared. > + } > + flow->packet_type = htonl(PT_MPLS); This can be moved to the if branch above. > + n = flow_count_mpls_labels(flow, ctx->wc); > + flow_push_mpls(flow, n, htons(ether_type), ctx->wc, true); Personally I would get ride of the n variable, but thats just a nit. + flow_push_mpls(flow, flow_count_mpls_labels(flow, ctx->wc), + htons(ether_type), ctx->wc, true); > + flow->dl_src = eth_addr_zero; > + flow->dl_dst = eth_addr_zero; > + Remove extra new line > +} > + > > /* For an MD2 NSH header returns a pointer to an ofpbuf with the encoded > * MD2 TLVs provided as encap properties to the encap operation. This > @@ -6535,6 +6579,12 @@ xlate_generic_encap_action(struct xlate_ctx *ctx, > case PT_NSH: > encap_data = rewrite_flow_push_nsh(ctx, encap, flow, wc); > break; > + case PT_MPLS: > + rewrite_flow_encap_mpls(ctx, encap, flow, wc); > + if (!ctx->xbridge->support.add_mpls) { > + ctx->xout->slow |= SLOW_ACTION; > + } > + break; > default: > /* New packet type was checked during decoding. */ > OVS_NOT_REACHED(); > @@ -6606,6 +6656,30 @@ xlate_generic_decap_action(struct xlate_ctx *ctx, > ctx->pending_decap = true; > /* Trigger recirculation. */ > return true; > + case PT_MPLS: { > + int n; > + ovs_be16 ethertype; > + > + flow->packet_type = decap->new_pkt_type; > + ethertype = pt_ns_type_be(flow->packet_type); > + > + n = flow_count_mpls_labels(flow, ctx->wc); > + if (!ethertype) { > + ethertype = htons(ETH_TYPE_TEB); > + } > + flow_pop_mpls(flow, n, ethertype, ctx->wc); > + > + if (!ctx->xbridge->support.add_mpls) { > + ctx->xout->slow |= SLOW_ACTION; > + } > + ctx->pending_decap = true; > + if (n == 1) { > + /* Trigger recirculation. */ > + return true; > + } else { > + return false; > + } > + } > default: > /* Error handling: drop packet. */ > xlate_report_debug( > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index cba49a99e..7e48eb12d 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -1538,6 +1538,44 @@ check_nd_extensions(struct dpif_backer *backer) > > return !error; > } > +/* Tests whether 'backer''s datapath supports the > + * OVS_ACTION_ATTR_ADD_MPLS action. */ > +static bool > +check_add_mpls(struct dpif_backer *backer) > +{ > + struct odputil_keybuf keybuf; > + struct ofpbuf actions; > + struct ofpbuf key; > + struct flow flow; > + bool supported; > + > + struct odp_flow_key_parms odp_parms = { > + .flow = &flow, > + .probe = true, > + }; > + > + memset(&flow, 0, sizeof flow); > + ofpbuf_use_stack(&key, &keybuf, sizeof keybuf); > + odp_flow_key_from_flow(&odp_parms, &key); > + ofpbuf_init(&actions, 64); > + > + struct ovs_action_add_mpls *mpls; > + > + mpls = nl_msg_put_unspec_zero(&actions, > + OVS_ACTION_ATTR_ADD_MPLS, > + sizeof *mpls); > + mpls->mpls_ethertype = htons(ETH_TYPE_MPLS); > + > + supported = dpif_probe_feature(backer->dpif, "add_mpls", &key, > + &actions, NULL); > + ofpbuf_uninit(&actions); > + VLOG_INFO("%s: Datapath %s add_mpls action", > + dpif_name(backer->dpif), supported ? "supports" > + : "does not support"); I would also move this as follows: + VLOG_INFO("%s: Datapath %s add_mpls action", + dpif_name(backer->dpif), + supported ? "supports" : "does not support"); > + return supported; > + Remove extra empty line > +} Can we also make sure the support table is updated, i.e update the get_datapath_cap() function. > + Remove extra empty line > > #define CHECK_FEATURE__(NAME, SUPPORT, FIELD, VALUE, ETHTYPE) \ > static bool \ > @@ -1609,6 +1647,7 @@ check_support(struct dpif_backer *backer) > backer->rt_support.lb_output_action = > dpif_supports_lb_output_action(backer->dpif); > backer->rt_support.ct_zero_snat = dpif_supports_ct_zero_snat(backer); > + backer->rt_support.add_mpls = check_add_mpls(backer); > > /* Flow fields. */ > backer->rt_support.odp.ct_state = check_ct_state(backer); > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index 191cfcb0d..6ce534ad6 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -207,7 +207,9 @@ struct group_dpif *group_dpif_lookup(struct ofproto_dpif > *, > DPIF_SUPPORT_FIELD(bool, lb_output_action, "Optimized Balance TCP mode")\ > \ > /* True if the datapath supports all-zero IP SNAT. */ \ > - DPIF_SUPPORT_FIELD(bool, ct_zero_snat, "Conntrack all-zero IP SNAT") > + DPIF_SUPPORT_FIELD(bool, ct_zero_snat, "Conntrack all-zero IP SNAT") \ > + /* True if the datapath supports layer 2 MPLS tunnelling */ \ Guess the description is not correct, as this is what the kernel header tells us: * @OVS_ACTION_ATTR_ADD_MPLS: Push a new MPLS label stack entry at the * start of the packet or at the start of the l3 header depending on the value * of l3 tunnel flag in the tun_flags field of OVS_ACTION_ATTR_ADD_MPLS * argument. > + DPIF_SUPPORT_FIELD(bool, add_mpls, "l2 MPLS tunnelling") I would simply change this to: "MPLS label add") > > > /* Stores the various features which the corresponding backer supports. */ > diff --git a/tests/mpls-xlate.at b/tests/mpls-xlate.at > index aafa89ba6..f8ba1fe87 100644 > --- a/tests/mpls-xlate.at > +++ b/tests/mpls-xlate.at > @@ -207,3 +207,50 @@ AT_CHECK([tail -1 stdout], [0], > > OVS_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([Encap Decap MPLS xlate action]) > + > +OVS_VSWITCHD_START( > + [add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1 -- \ > + add-port br0 p2 -- set Interface p2 type=patch \ > + options:peer=p3 ofport_request=2 -- \ > + add-br br1 -- \ > + set bridge br1 other-config:hwaddr=aa:66:aa:66:00:00 -- \ > + set bridge br1 datapath-type=dummy other-config:datapath-id=1234 \ > + fail-mode=secure -- \ > + add-port br1 p3 -- set Interface p3 type=patch \ > + options:peer=p2 ofport_request=3 -- \ > + add-port br1 p4 -- set Interface p4 type=dummy ofport_request=4]) > + > +AT_CHECK([ovs-appctl dpif/show], [0], [dnl > +dummy@ovs-dummy: hit:0 missed:0 > + br0: > + br0 65534/100: (dummy-internal) > + p1 1/1: (dummy) > + p2 2/none: (patch: peer=p3) > + br1: > + br1 65534/101: (dummy-internal) > + p3 3/none: (patch: peer=p2) > + p4 4/4: (dummy) > +]) > + > +AT_CHECK([ovs-ofctl del-flows br0]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br0 > "in_port=p1,actions=encap(mpls(ether_type=0x8847)),encap(ethernet),set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:00:00:01->dl_src,output:p2"]) > +AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br1 "in_port=p3,dl_type=0x8847 > actions=decap(),decap(packet_type(ns=0,type=0)),output:p4"]) > + > +# Now send two real ICMP echo request packets in on port p1 > + > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 > 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637] > ,[0], [ignore]) > + > +AT_CHECK([ovs-appctl netdev-dummy/receive p1 > 1e2ce92a669e3a6dd2099cab0800450000548a53400040011addc0a80a0ac0a80a1e08006f200a4d0001fc509a58000000002715020000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637] > ,[0], [ignore]) > + > +ovs-appctl time/warp 1000 > + > +AT_CHECK([ovs-appctl dpctl/dump-flows dummy@ovs-dummy | strip_used | grep -v > ipv6 |sort], [0], > +[flow-dump from the main thread: > +recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),eth_type(0x0800),ipv4(frag=no), > packets:1, bytes:98, used:0.0s, > actions:add_mpls(label=0,tc=0,ttl=64,bos=1,eth_type=0x8847),push_eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02),pop_eth,pop_mpls(eth_type=0x6558),recirc(0x1) > +recirc_id(0x1),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no), > packets:1, bytes:98, used:0.0s, actions:4 > +]) > + Can we do the same test with DP support disabled, to see it goes slow path, i.e., "ovs-appctl dpif/set-dp-features" > +OVS_VSWITCHD_STOP > +AT_CLEANUP > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index f400cfabc..b1f88b932 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -1081,9 +1081,236 @@ NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 > 10.1.1.2 | FORMAT_PING], [0], > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([mpls - encap header]) > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", 36:b1:ee:7c:01:03) > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", 36:b1:ee:7c:01:02) > + > +dnl The flow will encap a mpls header to the ip packet > +dnl eth/ip/icmp --> OVS --> eth/mpls/eth/ip/icmp > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 > "table=0,priority=100,dl_type=0x0800 > actions=encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:00:00:01->dl_src,ovs-p1"]) > + > +rm -rf p1.pcap > +NS_CHECK_EXEC([at_ns1], [tcpdump -l -n -xx -U -i p1 > p1.pcap &]) > +sleep 1 > + > +dnl The hex dump is a icmp packet. pkt=eth/ip/icmp > +dnl The packet is sent from p0(at_ns0) interface directed to > +dnl p1(at_ns1) interface > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 36 b1 ee 7c 01 02 36 > b1 ee 7c 01 03 08 00 45 00 00 54 03 44 40 00 40 01 21 61 0a 01 01 01 0a 01 01 > 02 08 00 ef ac 7c e4 00 03 5b 2c 1f 61 00 00 00 00 50 0b 02 00 00 00 00 00 10 > 11 12 13 14 15 16 17 18 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 28 29 2a > 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37 > /dev/null]) > + > +dnl Check the expected nsh encapsulated packet on the egress interface > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0000: *0000 *0000 *0002 *0000 *0000 > *0001 *8847 *0000" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0010: *2140 *36b1 *ee7c *0102 *36b1 > *ee7c *0103 *0800" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0020: *4500 *0054 *0344 *4000 *4001 > *2161 *0a01 *0101" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0030: *0a01 *0102 *0800 *efac *7ce4 > *0003 *5b2c *1f61" 2>&1 1>/dev/null]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + For the "mpls - encap header" above, and the decap header below we should have two tests. The one as is, but call it something like "mpls - encap header [DP support]" and skip it if the DP does not support it. And one called "mpls - encap header [slowpath]" and always run it with "ovs-appctl dpif/set-dp-features disabled" > +AT_SETUP([mpls - decap header]) > +OVS_TRAFFIC_VSWITCHD_START() > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", 36:b1:ee:7c:01:03) > +ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24", 36:b1:ee:7c:01:02) > + > +dnl The flow will decap a mpls header which in turn carries a icmp packet > +dnl eth/mpls/eth/ip/icmp --> OVS --> eth/ip/icmp > + > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flow br0 > "table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=decap(),decap(packet_type(ns=0,type=0)),ovs-p1"]) > + > +rm -rf p1.pcap > +NS_CHECK_EXEC([at_ns1], [tcpdump -l -n -xx -U -i p1 > p1.pcap &]) > +sleep 1 > + > +dnl The hex dump is an mpls packet encapsulating ethernet packet. > pkt=eth/mpls/eth/ip/icmp > +dnl The packet is sent from p0(at_ns0) interface directed to > +dnl p1(at_ns1) interface > +NS_CHECK_EXEC([at_ns0], [$PYTHON3 $srcdir/sendpkt.py p0 00 00 00 00 00 02 00 > 00 00 00 00 01 88 47 00 00 21 40 36 b1 ee 7c 01 02 36 b1 ee 7c 01 03 08 00 45 > 00 00 54 03 44 40 00 40 01 21 61 0a 01 01 01 0a 01 01 02 08 00 ef ac 7c e4 00 > 03 5b 2c 1f 61 00 00 00 00 50 0b 02 00 00 00 00 00 10 11 12 13 14 15 16 17 18 > 19 1a 1b 1c 1d 1e 1f 20 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 > 33 34 35 36 37 > /dev/null]) > + > +dnl Check the expected nsh encapsulated packet on the egress interface > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0000: *36b1 *ee7c *0102 *36b1 *ee7c > *0103 *0800 *4500" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0010: *0054 *0344 *4000 *4001 *2161 > *0a01 *0101 *0a01" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0020: *0102 *0800 *efac *7ce4 *0003 > *5b2c *1f61 *0000" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0030: *0000 *500b *0200 *0000 *0000 > *1011 *1213 *1415" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0040: *1617 *1819 *1a1b *1c1d *1e1f > *2021 *2223 *2425" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0050: *2627 *2829 *2a2b *2c2d *2e2f > *3031 *3233 *3435" 2>&1 1>/dev/null]) > +OVS_WAIT_UNTIL([cat p1.pcap | egrep "0x0060: *3637" 2>&1 1>/dev/null]) > + > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > + > +AT_SETUP([datapath - Encap Decap mpls actions]) > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24") > + > +AT_CHECK([ip link add patch0 type veth peer name patch1]) > +on_exit 'ip link del patch0' > + > +AT_CHECK([ip link set dev patch0 up]) > +AT_CHECK([ip link set dev patch1 up]) > +AT_CHECK([ovs-vsctl add-port br0 patch0 -- set interface patch0 > ofport_request=100]) > +AT_CHECK([ovs-vsctl add-port br1 patch1 -- set interface patch1 > ofport_request=100]) > + > +AT_DATA([flows.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:00:00:01->dl_src,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=decap(),decap(packet_type(ns=0,type=0)),resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) Can we change this as follows, so all traffic is going over the mpls tunnel? +table=0,priority=100,dl_type=0x0806 actions=encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:00:00:01->dl_src,output:100 table=0,priority=100,dl_type=0x8847,mpls_label=2 actions=decap(),decap(packet_type(ns=0,type=0)),resubmit(,3) -table=0,priority=10 actions=resubmit(,3) > +table=3,priority=10 actions=normal > +]) > + > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br1 flows.txt]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > + > +NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([datapath - multiple encap decap mpls actions]) > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24") > + > +AT_CHECK([ip link add patch0 type veth peer name patch1]) > +on_exit 'ip link del patch0' > + > +AT_CHECK([ip link set dev patch0 up]) > +AT_CHECK([ip link set dev patch1 up]) > +AT_CHECK([ovs-vsctl add-port br0 patch0 -- set interface patch0 > ofport_request=100]) > +AT_CHECK([ovs-vsctl add-port br1 patch1 -- set interface patch1 > ofport_request=100]) > + > +AT_DATA([flows.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=encap(mpls(ether_type=0x8847)),set_mpls_label:3, > encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:00:00:01->dl_src,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=decap(),decap(packet_type(ns=1,type=0x8847)),decap(packet_type(ns=0,type=0)),resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) Same as above, remove general traffic rule, and move all traffic over MPLS > +table=3,priority=10 actions=normal > +]) > + > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br1 flows.txt]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > + > NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], > [0], [dnl > 3 packets transmitted, 3 received, 0% packet loss, time 0ms > ]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([datapath - encap mpls pop mpls actions]) > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", 36:b1:ee:7c:01:03) > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24", 36:b1:ee:7c:01:02) > + > +AT_CHECK([ip link add patch0 type veth peer name patch1]) > +on_exit 'ip link del patch0' > + > +AT_CHECK([ip link set dev patch0 up]) > +AT_CHECK([ip link set dev patch1 up]) > +AT_CHECK([ovs-vsctl add-port br0 patch0 -- set interface patch0 > ofport_request=100]) > +AT_CHECK([ovs-vsctl add-port br1 patch1 -- set interface patch1 > ofport_request=100]) > + > +AT_DATA([flows.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=decap,encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),mod_dl_dst:36:b1:ee:7c:01:02,mod_dl_src:36:b1:ee:7c:01:03,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=pop_mpls:0x0800,resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) > +table=3,priority=10 actions=normal > +]) Same as earlier, remove general traffic rule (in flows and flows1), and move all traffic over MPLS > +AT_DATA([flows1.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=decap,encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),mod_dl_dst:36:b1:ee:7c:01:03,mod_dl_src:36:b1:ee:7c:01:02,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=pop_mpls:0x0800,resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) > +table=3,priority=10 actions=normal > +]) > + > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br1 flows1.txt]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +OVS_TRAFFIC_VSWITCHD_STOP > +AT_CLEANUP > + > +AT_SETUP([datapath - push mpls decap mpls actions]) > +OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > + > +ADD_NAMESPACES(at_ns0, at_ns1) > + > +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24", 36:b1:ee:7c:01:03) > +ADD_VETH(p1, at_ns1, br1, "10.1.1.2/24", 36:b1:ee:7c:01:02) > + > +AT_CHECK([ip link add patch0 type veth peer name patch1]) > +on_exit 'ip link del patch0' > + > +AT_CHECK([ip link set dev patch0 up]) > +AT_CHECK([ip link set dev patch1 up]) > +AT_CHECK([ovs-vsctl add-port br0 patch0 -- set interface patch0 > ofport_request=100]) > +AT_CHECK([ovs-vsctl add-port br1 patch1 -- set interface patch1 > ofport_request=100]) > + > +AT_DATA([flows.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=push_mpls:0x8847,set_field:2->mpls_label,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=decap,decap(packet_type(ns=1,type=0x0800)),encap(ethernet),mod_dl_dst:36:b1:ee:7c:01:03,mod_dl_src:36:b1:ee:7c:01:02,resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) > +table=3,priority=10 actions=normal > +]) Same as earlier, remove general traffic rule (in flows and flows1), and move all traffic over MPLS > +AT_DATA([flows1.txt], [dnl > +table=0,priority=100,dl_type=0x0800 > actions=push_mpls:0x8847,set_field:2->mpls_label,output:100 > +table=0,priority=100,dl_type=0x8847,mpls_label=2 > actions=decap,decap(packet_type(ns=1,type=0x0800)),encap(ethernet),mod_dl_dst:36:b1:ee:7c:01:02,mod_dl_src:36:b1:ee:7c:01:03,resubmit(,3) > +table=0,priority=10 actions=resubmit(,3) > +table=3,priority=10 actions=normal > +]) > + > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br0 flows.txt]) > +AT_CHECK([ovs-ofctl -Oopenflow13 add-flows br1 flows1.txt]) > + > +NS_CHECK_EXEC([at_ns0], [ping -q -c 3 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > +NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], > [0], [dnl > +3 packets transmitted, 3 received, 0% packet loss, time 0ms > +]) > + > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > > -- > 2.18.4 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
