On Wed, Apr 07, 2021 at 03:49:07PM +0000, Jan Scheurich wrote:
> Hi Martin,
>
> I guess you are aware of the original design document we wrote for generic
> encap/decap and NSH support:
> https://docs.google.com/document/d/1oWMYUH8sjZJzWa72o2q9kU0N6pNE-rwZcLH3-kbbDR8/edit#
>
> It is no longer 100% aligned with the final implementation in OVS but still a
> good reference for understanding the design principles behind the
> implementation and some specifics for Ethernet and NSH encap/decap use cases.
>
> Please find some more answers/comments below.
>
> BR, Jan
>
> > -----Original Message-----
> > From: Martin Varghese <[email protected]>
> > Sent: Wednesday, 7 April, 2021 10:43
> > To: Jan Scheurich <[email protected]>
> > Cc: Eelco Chaudron <[email protected]>; [email protected];
> > [email protected]; [email protected]
> > Subject: Re: [PATCH v4 1/2] Encap & Decap actions for MPLS packet type.
> >
> > On Tue, Apr 06, 2021 at 09:00:16AM +0000, Jan Scheurich wrote:
> > > Hi,
> > >
> > > Thanks for the heads up. The interaction with MPLS push/pop is a use case
> > that was likely not tested during the NSH and generic encap/decap design.
> > It's
> > complex code and a long time ago. I'm willing to help, but I will need some
> > time to go back and have a look.
> > >
> > > It would definitely help, if you could provide a minimal example for
> > reproducing the problem.
> > >
> >
> > Hi Jan ,
> >
> > Thanks for your help.
> >
> > I was trying to implement ENCAP/DECAP support for MPLS.
> >
> > The programming of datapath flow for the below userspace rule fails as
> > there
> > is set(eth() action between pop_mpls and recirc ovs-ofctl -O OpenFlow13 add-
> > flow br_mpls2 "in_port=$egress_port,dl_type=0x8847
> > actions=decap(),decap(packet_type(ns=0,type=0),goto_table:1
> >
> > 2021-04-05T05:46:49.192Z|00068|dpif(handler51)|WARN|system@ovs-
> > system: failed to put[create] (Invalid argument) ufid:1dddb0ba-27fe-44ea-
> > 9a99-5815764b4b9c
> > recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(6),skb_mark(0/0),ct_state
> > (0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0),eth(src=00:00:00:00:00:01/00:
> > 00:00:00:00:00,dst=00:00:00:00:00:02/00:00:00:00:00:00),eth_type(0x8847)
> > ,mpls(label=2/0x0,tc=0/0,ttl=64/0x0,bos=1/1),
> > actions:pop_eth,pop_mpls(eth_type=0x6558),set(eth()),recirc(0x45)
> >
>
> Conceptually, what should happen in this scenario is that, after the second
> decap(packet_type(ns=0,type=0) action, OVS processes the unchanged inner
> packet as packet type PT_ETH, i.e. as L2 Ethernet frame. Overwriting the
> existing Ethernet header with zero values through set(eth()) is clearly
> incorrect. That is a logical error inside the ofproto-dpif-xlate module (see
> below).
>
> I believe the netdev userspace datapath would still have accepted the
> incorrect datapath flow. I have too little experience with the kernel
> datapath to explain why that rejects the datapath flow as invalid.
>
> Unlike in the Ethernet and NSH cases, the MPLS header does not contain any
> indication about the inner packet type. That is why the packet_type must be
> provided by the SDN controller as part of the decap() action. And the
> ofproto-dpif-xlate module must consider the specified inner packet type when
> continuing the translation. In the general case, a decap() action should
> trigger recirculation for reparsing of the inner packet, so the new packet
> type must be set before recirculation. (Exceptions to the general
> recirculation rule are those where OVS has already parsed further into the
> packet and ofproto can modify the flow on the fly: decap(Ethernet) and
> possibly decap(MPLS) for all but the last bottom of stack label).
>
> I have had a look at your new code for encap/decap of MPLS headers, but I
> must admit I cannot fully judge in how far re-using the existing translation
> functions for MPLS label stacks written for the legacy push/pop_mpls case
> (i.e. manipulating a label stack between the L2 and the L3 headers of a
> PT_ETH Packet) are possible to re-use in the new context.
>
> BTW: Do you support multiple MPLS label encap or decap actions with your
> patch? Have you tested that?
>
> I am uncertain about the handling of the ethertype of the decapsulated inner
> packet. In the design base, the ethertype that is set in the existing L2
> header of the packet after pop_mpls of the last label is coming from the
> pop_mpls action, while in the decap(packet_type(0,0)) case the entire inner
> packet should be recirculated as is with packet_type PT_ETH.
>
> 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);
> flow_pop_mpls(flow, n, ethertype, ctx->wc);
> if (!ctx->xbridge->support.add_mpls) {
> ctx->xout->slow |= SLOW_ACTION;
> }
> ctx->pending_decap = true;
> return true;
>
> In the example scenario the new_pkt_type is PT_ETH, so the ethertype and
> hence flow->dl_type will be also be set to zero. Is that intentional? For
> target packet types of form PACKET_TYPE(OFPHTN_ETHERTYPE, <ethertype>) you
> would set the correct flow->dl_type but does this matter just before
> recirculation?
>
> I don't see any logic to check if the popped label was the BoS label. Only
> when popping the BoS label, we should recirculate the packet for parsing the
> inner packet.
>
> Conversely, in the encap(mpls) action, the new PT should be PT_MPLS and the
> all other flow layers (L2, L3, L4) should be cleared. I think the reused
> flow_push_mpls() function doesn't take care of clearing the L2 flow fields.
>
> Have you tested with other packet_types for inner packets (e.g. PT_IP or
> PT_ARP). If this is supposed to work, it would be nice to test the
> combination of
>
> decap(), encap(mpls), set_mpls_label, encap(eth), set_field:dl_dst
>
> on one side against the equivalent legacy action pop_mpls on the other end
> and vice versa.
>
>
> > As I pointed out in my previous mail, in commit_set_ether_action Function
> > “flow->packet_type != htonl(PT_ETH)” is used to check if the packet is
> > ethernet
> > instead of base_flow->packet_type.
> >
> > The mac address in flow structure is not cleared in PT_ETH handling of
> > xlate_generic_decap_action but it is cleared in base_flow in the decap
> > handling of PT_ETH in commit_encap_decap_action function Due to this
> > difference the set(eth(dst) action will be programmed to the datapath.
> >
> > I tried to fix this issue in 2 different ways.
> > 1 I have cleared the mac address in flow structure in PT_ETH handling of
> > xlate_generic_decap action.
> > 2 In the commit_set_ether action I changed the check from “flow-
> > >packet_type != htonl(PT_ETH)” to “base_flow->packet_type !=
> > htonl(PT_ETH))”.
>
> In the light of the discussion, I think you should implement both fixes as
> follows:
>
> 1. Clear the mac addresses in struct flow in the PT_ETH clause of
> xlate_generic_decap_action().
>
> This is correct as the flow struct is modified on the fly here and should
> accurately reflect the current packet headers.
>
> 2. Change the guard in commit_set_ether_action() to
>
> if (flow->packet_type != htonl(PT_ETH) || base_flow->packet_type !=
> htonl(PT_ETH)) {
> return;
> }
>
Do we need to check for both flow->packet_type & base_flow->packet_type?
I understand we just need to check only for "base_flow->packet_type !=
htonl(PT_ETH))". Could you please check again
> In either case setting the MAC header based on differences between base_flow
> and flow doesn't make sense.
>
> >
> > Though both of them solves this problem, couple of NSH regression tests are
> > failing
> >
> > 2291: nsh - md1 encap over a veth link FAILED (nsh.at:85)
> > 58022292: nsh - md2 encap over a veth link FAILED
> > (nsh.at:213)
> >
> > I see that they are failing as they are expecting a set(eth(dst) between
> > the the
> > pop_nsh and the recirc.
> > Set(eth) action is because of the reasons explained above – Datapath
> > actions:
> > push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x11223344,c
> > 2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:33:44:55:6
> > 6),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x1)
> >
> > In my understanding set(eth) here is wrong as there is no set ethernet
> > action
> > in the userspace rule
> > table=0,in_port=4,dl_type=0x894f,nsh_mdtype=1,nsh_spi=0x1234,nsh_c1=0x
> > 11223344,actions=decap(),decap(),2.
>
> I think you are right here. The extra set(eth(dst)) action is a bad
> translation artefact triggered by the above bugs.
>
> The failing basic NSH test cases carry Ethernet packets inside the NSH tunnel
> encap, so the decap() action should just recirculate the unchanged packets
> with PT_ETH.
>
> These test cases should be corrected as part of the bug-fix commit for the
> set(eth()) issue.
>
> >
> > To reproduce the mpls problem you could use the same test what I have added
> > in the patch. But you have to modify the test script like below
> > - table=0,priority=100,dl_type=0x0800
> > actions=encap(mpls(ether_type=0x8847)),set_mpls_label:2,encap(ethernet),o
> > utput:100
> > + 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"
> >
> >
> > > BR, Jan
> > >
> > > > -----Original Message-----
> > > > From: Eelco Chaudron <[email protected]>
> > > > Sent: Tuesday, 6 April, 2021 10:55
> > > > To: Martin Varghese <[email protected]>; Jan Scheurich
> > > > <[email protected]>
> > > > Cc: [email protected]; [email protected]; [email protected]
> > > > Subject: Re: [PATCH v4 1/2] Encap & Decap actions for MPLS packet type.
> > > >
> > > >
> > > >
> > > > On 6 Apr 2021, at 10:27, Martin Varghese wrote:
> > > >
> > > > > On Thu, Apr 01, 2021 at 11:32:06AM +0200, Eelco Chaudron wrote:
> > > > >>
> > > > >>
> > > > >> On 1 Apr 2021, at 11:28, Martin Varghese wrote:
> > > > >>
> > > > >>> On Thu, Apr 01, 2021 at 11:17:14AM +0200, Eelco Chaudron wrote:
> > > > >>>>
> > > > >>>>
> > > > >>>> On 1 Apr 2021, at 11:09, Martin Varghese wrote:
> > > > >>>>
> > > > >>>>> On Thu, Apr 01, 2021 at 10:54:42AM +0200, Eelco Chaudron wrote:
> > > > >>>>>>
> > > > >>>>>>
> > > > >>>>>> On 1 Apr 2021, at 10:35, Martin Varghese wrote:
> > > > >>>>>>
> > > > >>>>>>> On Thu, Apr 01, 2021 at 08:59:27AM +0200, Eelco Chaudron
> > wrote:
> > > > >>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>> On 1 Apr 2021, at 6:10, Martin Varghese wrote:
> > > > >>>>>>>>
> > > > >>>>>>>>> On Wed, Mar 31, 2021 at 03:59:40PM +0200, Eelco Chaudron
> > > > >>>>>>>>> wrote:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> On 26 Mar 2021, at 7:21, Martin Varghese wrote:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>> From: Martin Varghese <[email protected]>
> > > > >>>>>>>>>>>
> > > > >>>>>>>>>>> The encap & decap actions are extended to support MPLS
> > > > >>>>>>>>>>> packet type.
> > > > >>>>>>>>>>> Encap & decap actions adds and removes MPLS header at
> > > > >>>>>>>>>>> start of the packet.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Hi Martin,
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> I’m trying to do some real-life testing, and I’m running
> > > > >>>>>>>>>> into issues. This might be me setting it up wrongly but
> > > > >>>>>>>>>> just wanting to confirm…
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> I’m sending an MPLS packet that contains an ARP packet
> > > > >>>>>>>>>> into a physical port.
> > > > >>>>>>>>>> This is the packet:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Frame 4: 64 bytes on wire (512 bits), 64 bytes captured
> > > > >>>>>>>>>> (512
> > > > >>>>>>>>>> bits)
> > > > >>>>>>>>>> Encapsulation type: Ethernet (1)
> > > > >>>>>>>>>> [Protocols in frame: eth:ethertype:mpls:data]
> > > > >>>>>>>>>> Ethernet II, Src: 00:00:00_00:00:01 (00:00:00:00:00:01), Dst:
> > > > >>>>>>>>>> 00:00:00_00:00:02 (00:00:00:00:00:02)
> > > > >>>>>>>>>> Destination: 00:00:00_00:00:02 (00:00:00:00:00:02)
> > > > >>>>>>>>>> Address: 00:00:00_00:00:02 (00:00:00:00:00:02)
> > > > >>>>>>>>>> .... ..0. .... .... .... .... = LG bit: Globally
> > > > >>>>>>>>>> unique address (factory default)
> > > > >>>>>>>>>> .... ...0 .... .... .... .... = IG bit:
> > > > >>>>>>>>>> Individual address
> > > > >>>>>>>>>> (unicast)
> > > > >>>>>>>>>> Source: 00:00:00_00:00:01 (00:00:00:00:00:01)
> > > > >>>>>>>>>> Address: 00:00:00_00:00:01 (00:00:00:00:00:01)
> > > > >>>>>>>>>> .... ..0. .... .... .... .... = LG bit: Globally
> > > > >>>>>>>>>> unique address (factory default)
> > > > >>>>>>>>>> .... ...0 .... .... .... .... = IG bit:
> > > > >>>>>>>>>> Individual address
> > > > >>>>>>>>>> (unicast)
> > > > >>>>>>>>>> Type: MPLS label switched packet (0x8847)
> > > > >>>>>>>>>> MultiProtocol Label Switching Header, Label: 100, Exp: 0, S:
> > > > >>>>>>>>>> 1, TTL:
> > > > >>>>>>>>>> 64
> > > > >>>>>>>>>> 0000 0000 0000 0110 0100 .... .... .... = MPLS Label: 100
> > > > >>>>>>>>>> .... .... .... .... .... 000. .... .... = MPLS
> > > > >>>>>>>>>> Experimental
> > > > >>>>>>>>>> Bits: 0
> > > > >>>>>>>>>> .... .... .... .... .... ...1 .... .... = MPLS Bottom
> > > > >>>>>>>>>> Of Label
> > > > >>>>>>>>>> Stack: 1
> > > > >>>>>>>>>> .... .... .... .... .... .... 0100 0000 = MPLS TTL:
> > > > >>>>>>>>>> 64 Data (46 bytes)
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> 0000 ff ff ff ff ff ff 52 54 00 88 51 38 08 06 00 01
> > > > >>>>>>>>>> ......RT..Q8....
> > > > >>>>>>>>>> 0010 08 00 06 04 00 01 52 54 00 88 51 38 01 01 01 65
> > > > >>>>>>>>>> ......RT..Q8...e
> > > > >>>>>>>>>> 0020 00 00 00 00 00 00 01 01 01 64 27 98 a0 47
> > > > >>>>>>>>>> .........d'..G
> > > > >>>>>>>>>> Data:
> > > > >>>>>>>>>>
> > > >
> > ffffffffffff52540088513808060001080006040001525400885138010101650
> > 000
> > > > 0
> > > > 000?
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> I’m trying to use the following rules:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> ovs-ofctl del-flows ovs_pvp_br0
> > > > >>>>>>>>>> ovs-ofctl add-flow -O OpenFlow13 ovs_pvp_br0
> > > > >>>>>>>>>> "priority=100,dl_type=0x8847,mpls_label=100
> > > > >>>>>>>>>>
> > > > actions=decap(),decap(packet_type(ns=0,type=0x806)),resubmit(,3)"
> > > > >>>>>>>>>> ovs-ofctl add-flow -O OpenFlow13 ovs_pvp_br0
> > > > >>>>>>>>>> "table=3,priority=10
> > > > >>>>>>>>>> actions=normal"
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> With these, I expect the packet to be sent to vnet0, but
> > > > >>>>>>>>>> it’s not.
> > > > >>>>>>>>>> Actually,
> > > > >>>>>>>>>> the datapath rule looks odd, while the userspace rules
> > > > >>>>>>>>>> seem to match:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> $ ovs-dpctl dump-flows
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> recirc_id(0),in_port(1),eth(),eth_type(0x8847),mpls(label
> > > > >>>>>>>>>> =100 /0xfffff,tc=0/0,ttl=0/0x0,bos=1/1),
> > > > >>>>>>>>>> packets:13, bytes:1118, used:0.322s,
> > > > >>>>>>>>>> actions:pop_eth,pop_mpls(eth_type=0x806),recirc(0x19a)
> > > > >>>>>>>>>> recirc_id(0x19a),in_port(1),eth_type(0x0806),
> > > > >>>>>>>>>> packets:13, bytes:884, used:0.322s, actions:drop
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> $ ovs-ofctl dump-flows ovs_pvp_br0 -O OpenFlow13
> > > > >>>>>>>>>> cookie=0x0, duration=85.007s, table=0, n_packets=51,
> > > > >>>>>>>>>> n_bytes=4386,
> > > > >>>>>>>>>> priority=100,mpls,mpls_label=100
> > > > >>>>>>>>>>
> > > > actions=decap(),decap(packet_type(ns=0,type=0x806)),resubmit(,3)
> > > > >>>>>>>>>> cookie=0x0, duration=84.990s, table=3, n_packets=51,
> > > > >>>>>>>>>> n_bytes=3468,
> > > > >>>>>>>>>> priority=10 actions=NORMAL
> > > > >>>>>>>>>>
> > > > >>>>>>>>> The inner packet is ethernet. So the packet type should be
> > > > >>>>>>>>> (ns=0,type=0)
> > > > >>>>>>>>> ?
> > > > >>>>>>>>
> > > > >>>>>>>> Forgot to add that I already tried that to start with,
> > > > >>>>>>>> based on the example, but as that did not work I tried 0x806.
> > > > >>>>>>>>
> > > > >>>>>>>> PS: I have this as a remark in my review notes, i.e., to
> > > > >>>>>>>> explain the ns and type usage here.
> > > > >>>>>>>>
> > > > >>>>>>>>
> > > > >>>>>>>> This resulted in packets being counted at the open flow
> > > > >>>>>>>> level, but it results in NO data path rules. Do get an error
> > > > >>>>>>>> though:
> > > > >>>>>>>>
> > > > >>>>>>>> 2021-04-
> > > > 01T06:53:36.056Z|00141|dpif(handler37)|WARN|system@ovs-system:
> > > > >>>>>>>> failed to put[create] (Invalid argument)
> > > > >>>>>>>> ufid:3d2d6f6d-5a66-4ace-8b09-7cdcfa5efc8e
> > > > >>>>>>>> recirc_id(0),dp_hash(0/0),skb_priority(0/0),in_port(1),skb_
> > > > >>>>>>>> mark
> > > > >>>>>>>> (0/0),ct_state(0/0),ct_zone(0/0),ct_mark(0/0),ct_label(0/0)
> > > > >>>>>>>> ,eth
> > > > >>>>>>>> (src=00:00:00:00:00:01/00:00:00:00:00:00,dst=00:00:00:00:00
> > > > >>>>>>>> :02/
> > > > >>>>>>>> 00:00:00:00:00:00),eth_type(0x8847),mpls(label=100/0xfffff,
> > > > >>>>>>>> tc=0
> > > > >>>>>>>> /0,ttl=64/0x0,bos=1/1),
> > > > >>>>>>>> actions:pop_eth,pop_mpls(eth_type=0x6558),set(eth()),recirc
> > > > >>>>>>>> (0x4
> > > > >>>>>>>> c)
> > > > >>>>>>>
> > > > >>>>>>> This set(eth) before the recirc is the problem i guesss. I
> > > > >>>>>>> need to check
> > > > >
> > > > > I could reproduce the problem. It has nothing to do with ARP or IP.
> > > > > Unlike my test scripts, in your test you are setting the mac
> > > > > address after the encap action
> > > > >
> > > > > Ovs-vswitchd is programming a set(eth(dst) action between the
> > > > > pop_mpls and recirc as it sees a difference in mac address in
> > > > > flow structure and base_flow structure.
> > > > >
> > > > > The mac address in flow structure is not cleared in PT_ETH
> > > > > handling of xlate_generic_decap_action but it is cleared in
> > > > > base_flow in the decap handling of PT_ETH in
> > > > > commit_encap_decap_action function
> > > > >
> > > > > Due to this difference the set(eth(dst) action will be programmed
> > > > > to the datapath.
> > > > >
> > > > > Also, I see that in commit_set_ether_action Function
> > > > > “flow->packet_type != htonl(PT_ETH)” is used to check if the
> > > > > packet is ethernet instead of base_flow->packet_type.
> > > > >
> > > > > I assume check on base_flow->packet_type make more sense here ?
> > > > >
> > > > > I tried to fix this issue in 2 different ways.
> > > > >
> > > > > 1 I have cleared the mac address in flow structure in PT_ETH
> > > > > handling of xlate_generic_decap action.
> > > > >
> > > > > 2 In the commit_set_ether action I changed the check from
> > > > > “flow->packet_type != htonl(PT_ETH)” to “base_flow->packet_type
> > > > > != htonl(PT_ETH))”.
> > > > >
> > > > > Though both of them solves this problem, couple of NSH regression
> > > > > tests are failing
> > > > >
> > > > > 2291: nsh - md1 encap over a veth link FAILED
> > > > > (nsh.at:85)
> > > > >
> > > > > 58022292: nsh - md2 encap over a veth link FAILED
> > > > > (nsh.at:213)
> > > > >
> > > > > I see that they are failing as they are expecting a set(eth(dst)
> > > > > between the the pop_nsh and the recirc.
> > > > >
> > > > > Set(eth) action is because of the reasons explained above –
> > > > >
> > > > > Datapath actions:
> > > > > push_nsh(flags=0,ttl=63,mdtype=1,np=3,spi=0x1234,si=255,c1=0x11223
> > > > > 344,
> > > > > c2=0x0,c3=0x0,c4=0x0),push_eth(src=00:00:00:00:00:00,dst=11:22:33:
> > > > > 44:5
> > > > > 5:66),pop_eth,pop_nsh(),set(eth(dst=11:22:33:44:55:66)),recirc(0x1
> > > > > )
> > > > >
> > > > > In my understanding set(eth) here is wrong as there is no set
> > > > > ethernet action in the userspace rule
> > > > > - Hide quoted text -
> > > > >
> > > > > table=0,in_port=4,dl_type=0x894f,nsh_mdtype=1,nsh_spi=0x1234,nsh_c
> > > > > 1=0x
> > > > > 11223344,actions=decap(),decap(),2
> > > > >
> > > > >
> > > > >
> > > > > Could someone comment ?
> > > >
> > > > Maybe Jan can answer as he did the NSH implementation, however, what
> > > > would be of interest if you can give me an example of how the
> > > > encap()
> > > > decap() for this would be used in real life so I’m sure I’m testing it
> > correctly?
> > > >
> > > > What I did so far was to encapsulate all traffic going from a VM to
> > > > the physical port in MPLS using the flows like:
> > > >
> > > > ovs-ofctl add-flow -O OpenFlow13 ovs_pvp_br0
> > > > "priority=100,in_port=vnet0,actions=encap(mpls(ether_type=0x8847)),s
> > > > et_mpls
> > > > _label:100,encap(ethernet),,set_field:00:00:00:00:00:02->dl_ds
> > > > t,set_field:00:00:00:00:00:01->dl_src,output:enp5s0f0"
> > > >
> > > > Then I would capture this traffic and sent it back over the same
> > > > port, hoping it would come out as plane traffic with the following rule:
> > > >
> > > > ovs-ofctl add-flow -O OpenFlow15 ovs_pvp_br0
> > > > "priority=100,dl_type=0x8847,mpls_label=100
> > > > actions=decap(),decap(packet_type(ns=0,type=0)),resubmit(,3)"
> > > > ovs-ofctl add-flow -O OpenFlow15 ovs_pvp_br0 "table=3,priority=10
> > > > actions=normal"
> > > >
> > > > If this is correct, let me know, and if Jan does not reply, I’ll try
> > > > to understand the code in this area and see if I can find out some
> > > > details…
> > > >
> > > > //Eelco
> > > >
> > > > >>>>>>>> 2021-04-
> > > > 01T06:53:36.056Z|00142|dpif(handler37)|WARN|system@ovs-system:
> > > > >>>>>>>> execute
> > > > >>>>>>>> pop_eth,pop_mpls(eth_type=0x6558),set(eth()),recirc(0x4c)
> > > > >>>>>>>> failed
> > > > >>>>>>>> (Invalid argument) on packet
> > > > >>>>>>>> mpls,vlan_tci=0x0000,dl_src=00:00:00:00:00:01,dl_dst=00:00:
> > > > >>>>>>>> 00:0
> > > > >>>>>>>> 0:00:02,mpls_label=100,mpls_tc=0,mpls_ttl=64,mpls_bos=1
> > > > >>>>>>>> with metadata skb_priority(0),skb_mark(0),in_port(1) mtu 0
> > > > >>>>>>>>
> > > > >>>>>>>> Are there missing parts in my kernel that do not get
> > > > >>>>>>>> properly detected by the feature detection?
> > > > >>>>>>>>
> > > > >>>>>>>> $ ovs-appctl dpif/show-dp-features ovs_pvp_br0 Masked set
> > > > >>>>>>>> action: Yes Tunnel push pop: No
> > > > >>>>>>>> Ufid: Yes
> > > > >>>>>>>> Truncate action: Yes
> > > > >>>>>>>> Clone action: Yes
> > > > >>>>>>>> Sample nesting: 10
> > > > >>>>>>>> Conntrack eventmask: Yes
> > > > >>>>>>>> Conntrack clear: Yes
> > > > >>>>>>>> Max dp_hash algorithm: 0
> > > > >>>>>>>> Check pkt length action: Yes Conntrack timeout policy: Yes
> > > > >>>>>>>> Explicit Drop action: No Optimized Balance TCP mode: No
> > > > >>>>>>>> l2 MPLS tunnelling: Yes
> > > > >>>>>>>> Max VLAN headers: 2
> > > > >>>>>>>> Max MPLS depth: 3
> > > > >>>>>>>> Recirc: Yes
> > > > >>>>>>>> CT state: Yes
> > > > >>>>>>>> CT zone: Yes
> > > > >>>>>>>> CT mark: Yes
> > > > >>>>>>>> CT label: Yes
> > > > >>>>>>>> CT state NAT: Yes
> > > > >>>>>>>> CT orig tuple: Yes
> > > > >>>>>>>> CT orig tuple for IPv6: Yes
> > > > >>>>>>>> IPv6 ND Extension: No
> > > > >>>>>>>>
> > > > >>>>>>> You are good
> > > > >>>>>>>
> > > > >>>>>>> I am not sure what is going wrong. Your test case looks same as
> > > > >>>>>>> the unit test i added.
> > > > >>>>>>>
> > > > >>>>>>> I tried myself again and this is i get
> > > > >>>>>>>
> > > > >>>>>>> ovs-ofctl -O OpenFlow13 add-flow br_mpls2
> > > > >>>>>>> "in_port=$egress_port,dl_type=0x8847
> > > > >>>>>>> +actions=decap(),decap(packet_type(ns=0,type=0),goto_table:1"
> > > > >>>>>>> ovs-ofctl -O OpenFlow13 add-flow br_mpls2
> > > > >>>>>>> +"table=1,in_port=$egress_port,dl_type=0x0800,nw_dst=1.1.1.2
> > > > >>>>>>> +actions=set_field:00:00:00:00:00:02->dl_dst,set_field:00:00:00:
> > > > >>>>>>> +00:00:01->dl_sr
> > > > >>>>>>> +c output:$ingress_port"
> > > > >>>>>>>
> > > > >>>>>>> recirc_id(0x3),in_port(6),eth(src=36:b1:ee:7c:01:03,dst=36:b1:ee
> > > > >>>>>>> :7c:01:02),eth_
> > > > >>>>>>> +type(0x0800),ipv4(dst=1.1.1.2,frag=no), packets:3, bytes:294,
> > > > >>>>>>> used:0.837s,
> > > > >>>>>>>
> > +actions:set(eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:02)),4
> > > > >>>>>>> recirc_id(0),in_port(6),eth(),eth_type(0x8847),mpls(label=0/0x0,
> > > > >>>>>>> tc=0/0,ttl=0/0x
> > > > >>>>>>> +0,bos=1/1), packets:3, bytes:348, used:0.837s,
> > > > >>>>>>> +actions:pop_eth,pop_mpls(eth_type=0x6558),recirc(0x3)
> > > > >>>>>>>
> > > > >>>>>>> The packet to the ovs is
> > > > >>>>>>> ETH|MPLS|ETH|IP ?
> > > > >>>>>>> How it is differnt from you test case?
> > > > >>>>>>
> > > > >>>>>> Mine is ETH|MPLS|ETH|ARP, which works fine with pop_mpls
> > > > >>>>>>
> > > > >>>>> I am wondering how old mpls pop action works could you please put
> > > > >>>>> down the userspace and datapath rules when you used pop_mpls.
> > > > >>>>>
> > > > >>>>> In my understanding it can never work as what you have after MPLS
> > > > >>>>> is ethernet and not ARP.
> > > > >>>>
> > > > >>>> It’s ethernet + ARP, but here are my rules:
> > > > >>>
> > > > >>> To clarify
> > > > >>>
> > > > >>> The test vector for Decap Test case
> > > > >>> ETH|MPLS|ETH|ARP
> > > > >>> The test vector for pop mpls test case
> > > > >>> ETH|MPLS|ARP|
> > > > >>>
> > > > >>> The above understanding correct?
> > > > >>
> > > > >> Guess our emails crossed ;) I was sending in the same packet for
> > > > >> both the test cases, so
> > > > >>
> > > > >> ETH|MPLS|ETH|ARP
> > > > >>
> > > > >> Which with decap() is resulting in the rules not being programmed
> > > > >>
> > > > >> With popmpls I saw the packets in being received, but did not notice
> > > > >> the incorrect use of popmpls so my packet after popmpls looks like
> > > > >> ETH|ETH|ARP.
> > > > >>
> > > > >> So I guess all that remains is why the data path rules is not
> > > > >> accepted for ARP with the decap.
> > > > >>
> > > > >>>>
> > > > >>>> dpctl:
> > > > >>>>
> > > > >>>> recirc_id(0),in_port(2),eth(),eth_type(0x8847),mpls(label=100/0xfff
> > > > >>>> ff,tc=0/0,ttl=0/0x0,bos=1/1), packets:64, bytes:5504, used:0.444s,
> > > > >>>> actions:pop_mpls(eth_type=0x806),recirc(0x80d)
> > > > >>>> recirc_id(0x80d),in_port(2),eth(src=00:00:00:00:00:01,dst=00:00:00:
> > > > >>>> 00:00:02),eth_type(0x0806), packets:64, bytes:5248, used:0.444s,
> > > > >>>> actions:3,1
> > > > >>>>
> > > > >>>> ofctl:
> > > > >>>>
> > > > >>>> OFPST_FLOW reply (OF1.5) (xid=0x2):
> > > > >>>> cookie=0x0, duration=178.890s, table=0, n_packets=127,
> > > > >>>> n_bytes=10922, idle_age=0, priority=100,mpls,mpls_label=100
> > > > >>>> actions=pop_mpls:0x0806,resubmit(,3)
> > > > >>>> cookie=0x0, duration=178.873s, table=3, n_packets=127,
> > > > >>>> n_bytes=10414, idle_age=0, priority=10 actions=NORMAL
> > > > >>>>
> > > > >>>>
> > > > >>>>>>> Thanks for your time.
> > > > >>>>>>
> > > > >>>>>> Your welcome
> > > > >>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> If I use the old way, doing pop_mpls, it works fine:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> ovs-ofctl del-flows ovs_pvp_br0 ovs-ofctl add-flow -O
> > > > >>>>>>>>>> OpenFlow13 ovs_pvp_br0
> > > > >>>>>>>>>> "priority=100,dl_type=0x8847,mpls_label=100
> > > > >>>>>>>>>> actions=pop_mpls:0x0806,resubmit(,3)"
> > > > >>>>>>>>>> ovs-ofctl add-flow -O OpenFlow13 ovs_pvp_br0
> > > > >>>>>>>>>> "table=3,priority=10
> > > > >>>>>>>>>> actions=normal"
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> I also noticed (despite the test example) to make encap work,
> > > > >>>>>>>>>> I had to set the ethernet MAC addresses, or else the packets
> > > > >>>>>>>>>> were not getting out.
> > > > >>>>>>>>>> So something like:
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> ovs-ofctl add-flow -O OpenFlow13
> > > > >>>>>>>>>> ovs_pvp_br0
> > > > >>>>>>>>>>
> > "priority=100,in_port=vnet0,actions=encap(mpls(ether_type=0x8
> > > > >>>>>>>>>> 847)),set_mpls_label:100,encap(ethernet),,set_field:00:00:00:
> > > > >>>>>>>>>> 00:00:02->dl_dst,set_field:00:00:00:00:00:01-
> > >dl_src,output:e
> > > > >>>>>>>>>> np5s0f0
> > > > >>>>>>>>>>
> > > > >>>>>>>
> > > > >>>>>>>>>
> > > > >>>>>>>>> The packets are not going out because you are sending the
> > > > >>>>>>>>> packet on a real nic and not on a virtual inerface (veth pair)
> > > > >>>>>>>>> ?
> > > > >>>>>>>>
> > > > >>>>>>>> So for a real NIC we need to set the MAC addresses, maybe some
> > > > >>>>>>>> where in the documentation we should add an example on how
> > to
> > > > >>>>>>>> use this feature?
> > > > >>>>>>>>
> > > > >>>>>>>>>> Maybe the test case can be made more realistic? Once I
> > > > >>>>>>>>>> understand the failure, I can continue with the review.
> > > > >>>>>>>>>>
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Cheers,
> > > > >>>>>>>>>>
> > > > >>>>>>>>>> Eelco
> > >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
