mbox series

[ovs-dev,v3,0/2] Encap & Decap actions for MPLS packet type

Message ID cover.1588869591.git.martin.varghese@nokia.com
Headers show
Series Encap & Decap actions for MPLS packet type | expand

Message

Martin Varghese May 8, 2020, 4:30 a.m. UTC
From: Martin Varghese <martin.varghese@nokia.com>

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

Martin Varghese (2):
  Datapath: New MPLS actions for layer 2 tunnelling
  Encap & Decap actions for MPLS packet type

 NEWS                                              |  1 +
 datapath/actions.c                                | 42 ++++++++---
 datapath/flow_netlink.c                           | 33 +++++++++
 datapath/linux/compat/include/linux/openvswitch.h | 35 ++++++++-
 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                                    | 38 ++++++++--
 lib/ofp-actions.c                                 |  5 ++
 lib/ofp-ed-props.c                                | 89 +++++++++++++++++++++++
 lib/ovs-actions.xml                               | 32 ++++++--
 lib/packets.c                                     | 36 +++++++++
 lib/packets.h                                     |  2 +
 ofproto/ofproto-dpif-ipfix.c                      |  1 +
 ofproto/ofproto-dpif-sflow.c                      |  1 +
 ofproto/ofproto-dpif-xlate.c                      | 54 ++++++++++++++
 tests/system-traffic.at                           | 34 +++++++++
 18 files changed, 410 insertions(+), 25 deletions(-)

Comments

Eelco Chaudron Nov. 20, 2020, 1:01 p.m. UTC | #1
Hi Martin,

I planned to do a full review of this patchset, which I usually start 
with testing. However, during my testing, I found that this patch is not 
backward compatible. If I try to run this on a kernel without the kmod 
change, the existing push_mpls action fails.

On top of this, I think that if the kernel does not support the 
OVS_ACTION_ATTR_ADD_MPLS action, it should fallback using the 
SLOW_ACTION, so the packet gets processed in userspace.

In addition, I got a compiler warning on the following:

> @@ -180,6 +222,11 @@ 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;
> +        }

Either do a "return false" or an explicit fall trough to stop the 
compiler from complaining.

>      default:
>          return false;
>      }

So please fix the backward/forward compatibility stuff in the next 
version, and I'll do the full review then as I suspect there will be a 
bunch of related changes.

Also, make sure the patches apply and compile individually. See the 
comment from Ilya earlier.

Cheers,

Eelco

On 8 May 2020, at 6:30, Martin Varghese wrote:

> From: Martin Varghese <martin.varghese@nokia.com>
>
> 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
>
> Martin Varghese (2):
>   Datapath: New MPLS actions for layer 2 tunnelling
>   Encap & Decap actions for MPLS packet type
>
>  NEWS                                              |  1 +
>  datapath/actions.c                                | 42 ++++++++---
>  datapath/flow_netlink.c                           | 33 +++++++++
>  datapath/linux/compat/include/linux/openvswitch.h | 35 ++++++++-
>  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                                    | 38 ++++++++--
>  lib/ofp-actions.c                                 |  5 ++
>  lib/ofp-ed-props.c                                | 89 
> +++++++++++++++++++++++
>  lib/ovs-actions.xml                               | 32 ++++++--
>  lib/packets.c                                     | 36 +++++++++
>  lib/packets.h                                     |  2 +
>  ofproto/ofproto-dpif-ipfix.c                      |  1 +
>  ofproto/ofproto-dpif-sflow.c                      |  1 +
>  ofproto/ofproto-dpif-xlate.c                      | 54 ++++++++++++++
>  tests/system-traffic.at                           | 34 +++++++++
>  18 files changed, 410 insertions(+), 25 deletions(-)
>
> -- 
> 1.8.3.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Varghese, Martin (Nokia - IN/Bangalore) Nov. 20, 2020, 1:23 p.m. UTC | #2
Hi Eelco,

Thanks for taking time to review the patch.  I had noted the comments from Ilya, but could not finish doing the next version because of other commitments. I hope I will be able to send out the next version soon.

Regards,
Martin