| Message ID | 20210830124041.2749-1-martinvarghesenokia@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v5] Encap & Decap actions for MPLS packet type. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/apply-robot | fail | apply and check: fail |
Hi Martin, See my comments below... Cheers, Eelco On 30 Aug 2021, at 14:40, Martin Varghese wrote: > From: Martin Varghese <martin.varghese@nokia.com> > > 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 <martin.varghese@nokia.com> > --- > 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
Bleep bloop. Greetings Martin Varghese, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. git-am: error: Failed to merge in the changes. hint: Use 'git am --show-current-patch' to see the failed patch Patch failed at 0001 Encap & Decap actions for MPLS packet type. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". Patch skipped due to previous failure. Please check this out. If you feel there has been an error, please email aconole@redhat.com Thanks, 0-day Robot
On Fri, Sep 24, 2021 at 02:30:16PM +0200, Eelco Chaudron wrote: > Hi Martin, > > See my comments below... > > Cheers, > > Eelco > > On 30 Aug 2021, at 14:40, Martin Varghese wrote: > > > From: Martin Varghese <martin.varghese@nokia.com> > > > > 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 <martin.varghese@nokia.com> > > --- > > 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? > The above check is redundant. I will remove thie check in the next version. when ETH_TYE_TEB is configured as the ethertype in pop_mpls, it means the inner packet is ethernet. Not sure if a packet can have ETH_TYPE_TEB as the ethertype to support ethernet in ethernet. As I know ETH_TYPE_TEB is used with NSH encap. This piece of code get hit only for MPLS packets so it doesnot matter if packet with ETH_TYPE_TEB as ethertype really comes. > > + 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? > I will add a comment here. > > + > > + 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. > This is to handle case where the incoming packet is already a MPLS packet. I will add a comment. > > + } > > + 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. In this case we can skip the test only once the test is started. ie once the bridge is created to fetch the DP features. I have not see this approach being used in any other tests and it appears unclean. Hence i prefer to let the test run in slow path if the datapath does not have the support > 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 > unlike previous 2 tests this is a layer 3 MPLS tunnelling test. So we cannot remove the generic rule. Or we need to add a seperate rule for ARP which generally not seen in a L3 VPN use case > > +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 > same as above > > +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 > Thanks Eelco for your time
Hi Martin, I see you already sent out a v6 before I could respond, so I will move/respond to the open items to the v6 review. This way, all the correspondence is in one thread. //Eelco On 28 Sep 2021, at 15:35, Martin Varghese wrote: > On Fri, Sep 24, 2021 at 02:30:16PM +0200, Eelco Chaudron wrote: >> Hi Martin, >> >> See my comments below... >> >> Cheers, >> >> Eelco >> >> On 30 Aug 2021, at 14:40, Martin Varghese wrote: >> >>> From: Martin Varghese <martin.varghese@nokia.com> >>> >>> 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 <martin.varghese@nokia.com> >>> --- >>> 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? >> > The above check is redundant. I will remove thie check in the next version. > > when ETH_TYE_TEB is configured as the ethertype in pop_mpls, it means > the inner packet is ethernet. Not sure if a packet can have ETH_TYPE_TEB > as the ethertype to support ethernet in ethernet. As I know ETH_TYPE_TEB > is used with NSH encap. > This piece of code get hit only for MPLS packets so it doesnot matter if > packet with ETH_TYPE_TEB as ethertype really comes. >>> + 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? >> > I will add a comment here. >>> + >>> + 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. >> > This is to handle case where the incoming packet is already a MPLS > packet. I will add a comment. >>> + } >>> + 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. > > In this case we can skip the test only once the test is started. ie once the > bridge is created to fetch the DP features. > I have not see this approach being used in any other tests and it > appears unclean. Hence i prefer to let the test run in slow path if the datapath does not have the support > >> 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 >> > unlike previous 2 tests this is a layer 3 MPLS tunnelling test. So we > cannot remove the generic rule. Or we need to add a seperate rule for > ARP which generally not seen in a L3 VPN use case >>> +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 >> > same as above >>> +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 >> > > Thanks Eelco for your time
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 */ + 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 .*/ + 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)); + 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) { + case OFPPPT_PROP_MPLS_ETHERTYPE: { + struct ofp_ed_prop_mpls_ethertype *opnmt = + 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 = + 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) { + case OFPPPT_PROP_MPLS_ETHERTYPE: { + struct ofpact_ed_prop_mpls_ethertype *pnmt = + ALIGNED_CAST(struct ofpact_ed_prop_mpls_ethertype *, *prop); + struct ofp_ed_prop_mpls_ethertype *opnmt = + ofpbuf_put_uninit(out, sizeof(*opnmt)); + 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; + + } + 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)); + 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: + 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: + 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: + 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 @@ -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. + </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) +{ + char * header; + + if (!eth_type_mpls(ethtype)) { + return; + } + + if (!l3) { + header = dp_packet_push_uninit(packet, MPLS_HLEN); + 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)) { + 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)) { + set_ethertype(packet, ethtype); + } + 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); + + 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); /* 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++) { + 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); + } + flow->packet_type = htonl(PT_MPLS); + n = flow_count_mpls_labels(flow, ctx->wc); + flow_push_mpls(flow, n, htons(ether_type), ctx->wc, true); + flow->dl_src = eth_addr_zero; + flow->dl_dst = eth_addr_zero; + +} + /* 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"); + return supported; + +} + #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 */ \ + DPIF_SUPPORT_FIELD(bool, add_mpls, "l2 MPLS tunnelling") /* 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 +]) + +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 + +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) +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) +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 +]) + +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 +]) + +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