lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Thu, 7 Jul 2016 13:54:15 -0700
From:	pravin shelar <pshelar@....org>
To:	Simon Horman <simon.horman@...ronome.com>
Cc:	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	ovs dev <dev@...nvswitch.org>
Subject: Re: [ovs-dev] [PATCH net-next v11 5/6] openvswitch: add layer 3
 flow/port support

On Wed, Jul 6, 2016 at 10:59 AM, Simon Horman
<simon.horman@...ronome.com> wrote:
> From: Lorand Jakab <lojakab@...co.com>
>
> Implementation of the pop_eth and push_eth actions in the kernel, and
> layer 3 flow support.
>
> This doesn't actually do anything yet as no layer 2 tunnel ports are
> supported yet. The original patch by Lorand was against the Open vSwitch
> tree which has L2 LISP tunnels but that is not supported in mainline Linux.
> I (Simon) plan to follow up with support for non-TEB GRE ports based on
> work by Thomas Morin.
>
> Cc: Thomas Morin <thomas.morin@...nge.com>
> Signed-off-by: Lorand Jakab <lojakab@...co.com>
> Signed-off-by: Simon Horman <simon.horman@...ronome.com>
>
> ---
....
> ---
>  include/uapi/linux/openvswitch.h     |  11 ++
>  net/openvswitch/actions.c            |  45 ++++++++
>  net/openvswitch/datapath.c           |  13 +--
>  net/openvswitch/flow.c               |  65 +++++++----
>  net/openvswitch/flow.h               |   4 +-
>  net/openvswitch/flow_netlink.c       | 213 ++++++++++++++++++++++++-----------
>  net/openvswitch/vport-geneve.c       |   2 +-
>  net/openvswitch/vport-gre.c          |   2 +-
>  net/openvswitch/vport-internal_dev.c |   6 +
>  net/openvswitch/vport-netdev.c       |  19 +++-
>  net/openvswitch/vport-netdev.h       |   2 +
>  net/openvswitch/vport-vxlan.c        |   2 +-
>  12 files changed, 279 insertions(+), 105 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 5cde501433eb..6f505e486e93 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
...

> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 12e8a8942a42..0001f651c934 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -301,6 +301,43 @@ static int set_eth_addr(struct sk_buff *skb, struct sw_flow_key *flow_key,
>         return 0;
>  }
>
> +/* pop_eth does not support VLAN packets as this action is never called
> + * for them.
> + */
> +static int pop_eth(struct sk_buff *skb, struct sw_flow_key *key)
> +{
> +       skb_pull_rcsum(skb, ETH_HLEN);
> +       skb_reset_mac_header(skb);
> +       skb->mac_len -= ETH_HLEN;
> +
> +       invalidate_flow_key(key);
> +       return 0;
> +}
This is changing l2 packet to l3 packet by reseting mac header. We
need to unset mac header so that OVS key-extract can identify this
packet later on, for example after recirc action.
Other option would be keeping key is_layer3 consistent with packet.
Push ethernet and pop ethernet action can unset and set the flag in
flow key. So that OVS can keep track of packet headers by looking at
packet key. When packet leaves OVS (in netdev-send) we can unset mac
header according to this flag.

> +
> +static int push_eth(struct sk_buff *skb, struct sw_flow_key *key,
> +                   const struct ovs_action_push_eth *ethh)
> +{
> +       struct ethhdr *hdr;
> +
> +       /* Add the new Ethernet header */
> +       if (skb_cow_head(skb, ETH_HLEN) < 0)
> +               return -ENOMEM;
> +
> +       skb_push(skb, ETH_HLEN);
> +       skb_reset_mac_header(skb);
> +       skb->mac_len += ETH_HLEN;
> +
> +       hdr = eth_hdr(skb);
> +       ether_addr_copy(hdr->h_source, ethh->addresses.eth_src);
> +       ether_addr_copy(hdr->h_dest, ethh->addresses.eth_dst);
> +       hdr->h_proto = skb->protocol;
> +
> +       skb_postpush_rcsum(skb, hdr, ETH_HLEN);
> +
> +       invalidate_flow_key(key);
> +       return 0;
> +}
> +
....

> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 0ea128eeeab2..86f2cfb19de3 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
...

> @@ -723,9 +729,17 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
>         key->phy.skb_mark = skb->mark;
>         ovs_ct_fill_key(skb, key);
>         key->ovs_flow_hash = 0;
> +       key->phy.is_layer3 = skb->mac_len == 0;

I do not think mac_len can be used. mac_header needs to be checked.
...

> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index c78a6a1476fb..fc94fbe1ddc3 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
...

> @@ -898,20 +922,33 @@ static int metadata_from_nlattrs(struct net *net, struct sw_flow_match *match,
>                                    sizeof(*cl), is_mask);
>                 *attrs &= ~(1ULL << OVS_KEY_ATTR_CT_LABELS);
>         }
> -       return 0;
> -}
>
> -static int ovs_key_from_nlattrs(struct net *net, struct sw_flow_match *match,
> -                               u64 attrs, const struct nlattr **a,
> -                               bool is_mask, bool log)
> -{
> -       int err;
> +       /* For layer 3 packets the ethernet type is provided
> +        * and treated as metadata but no MAC addresses are provided.
> +        */
> +       if (*attrs & (1ULL << OVS_KEY_ATTR_ETHERTYPE) &&
> +           !(*attrs & (1 << OVS_KEY_ATTR_ETHERNET))) {
> +               int err;
>
Here attr_ethertype can be processed irrespective of attr_ethernet.
is_layer3 can be derived independently. This way there is no need to
again process attr_ethertyp in l2_from_nlattrs().

> -       err = metadata_from_nlattrs(net, match, &attrs, a, is_mask, log);
> -       if (err)
> -               return err;
> +               err = ethertype_from_nlattrs(net, match, attrs, a, is_mask,
> +                                            log);
> +               if (err)
> +                       return err;
> +
> +               is_layer3 = true;
> +       }
>
> -       if (attrs & (1 << OVS_KEY_ATTR_ETHERNET)) {
> +       /* Always exact match is_layer3 */
> +       SW_FLOW_KEY_PUT(match, phy.is_layer3, is_mask ? true : is_layer3,
> +                       is_mask);
> +       return is_layer3;
> +}
> +
....
> +       if (*attrs & (1 << OVS_KEY_ATTR_ETHERTYPE)) {
> +               int err;
>
> -               SW_FLOW_KEY_PUT(match, eth.type, eth_type, is_mask);
> -               attrs &= ~(1 << OVS_KEY_ATTR_ETHERTYPE);
> +               err = ethertype_from_nlattrs(net, match, attrs, a, is_mask,
> +                                            log);
> +               if (err)
> +                       return err;
>         } else if (!is_mask) {
>                 SW_FLOW_KEY_PUT(match, eth.type, htons(ETH_P_802_2), is_mask);
>         }
>
> +       return 0;
> +}
> +
> +static int ovs_key_from_nlattrs(struct net *net, struct sw_flow_match *match,
> +                               u64 attrs, const struct nlattr **a,
> +                               bool is_mask, bool log)
> +{
> +       int err;
> +       bool is_layer3;
> +
> +       err = metadata_from_nlattrs(net, match, &attrs, a, is_mask, log);
> +       if (err < 0)
> +               return err;
> +       is_layer3 = err != 0;
> +
> +       if (!is_layer3) {
> +               err = l2_from_nlattrs(net, match, &attrs, a, is_mask, log);
> +               if (err < 0)
> +                       return err;
> +       }
> +
...


> diff --git a/net/openvswitch/vport-internal_dev.c b/net/openvswitch/vport-internal_dev.c
> index 32d8e94d9bff..adc364161626 100644
> --- a/net/openvswitch/vport-internal_dev.c
> +++ b/net/openvswitch/vport-internal_dev.c
> @@ -257,6 +257,12 @@ static netdev_tx_t internal_dev_recv(struct sk_buff *skb)
>         struct net_device *netdev = skb->dev;
>         struct pcpu_sw_netstats *stats;
>
> +       /* Only send/receive L2 packets */
> +       if (!skb->mac_len) {
> +               kfree_skb(skb);
> +               return -EINVAL;
> +       }
> +
Is mac_len consistent? I thought we decided to use
skb_mac_header_was_set() to detect l3 packets.

>         if (unlikely(!(netdev->flags & IFF_UP))) {
>                 kfree_skb(skb);
>                 netdev->stats.rx_dropped++;
> diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
> index 4e3972344aa6..733e7914f6bd 100644
> --- a/net/openvswitch/vport-netdev.c
> +++ b/net/openvswitch/vport-netdev.c
> @@ -57,8 +57,10 @@ static void netdev_port_receive(struct sk_buff *skb)
>         if (unlikely(!skb))
>                 return;
>
> -       skb_push(skb, ETH_HLEN);
> -       skb_postpush_rcsum(skb, skb->data, ETH_HLEN);
> +       if (vport->dev->type == ARPHRD_ETHER) {
> +               skb_push(skb, ETH_HLEN);
> +               skb_postpush_rcsum(skb, skb->data, ETH_HLEN);
> +       }
This is still required for tunnel device of ARPHRD_NONE which can
handle l2 packets.

>         ovs_vport_receive(vport, skb, skb_tunnel_info(skb));
>         return;
>  error:
> @@ -194,6 +196,17 @@ void ovs_netdev_tunnel_destroy(struct vport *vport)
>  }
>  EXPORT_SYMBOL_GPL(ovs_netdev_tunnel_destroy);
>
> +int ovs_netdev_send(struct sk_buff *skb)
> +{
> +       /* Only send L2 packets */
> +       if (skb->mac_len)
> +               return dev_queue_xmit(skb);
> +
As commented in earlier, we can send is_layer3 flag from flow key. If
it is l3 packet then unset mac header before sending it out to keep
the packet metadata consistent.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ