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  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:   Fri, 4 Sep 2020 17:45:48 +0200
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Murali Karicheri <m-karicheri2@...com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Network Development <netdev@...r.kernel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>, nsekhar@...com,
        Grygorii Strashko <grygorii.strashko@...com>
Subject: Re: [PATCH net-next 1/1] net: hsr/prp: add vlan support

On Tue, Sep 1, 2020 at 9:54 PM Murali Karicheri <m-karicheri2@...com> wrote:
>
> This patch add support for creating vlan interfaces
> over hsr/prp interface.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@...com>
> ---
>  net/hsr/hsr_device.c  |  4 ----
>  net/hsr/hsr_forward.c | 16 +++++++++++++---
>  2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
> index ab953a1a0d6c..e1951579a3ad 100644
> --- a/net/hsr/hsr_device.c
> +++ b/net/hsr/hsr_device.c
> @@ -477,10 +477,6 @@ void hsr_dev_setup(struct net_device *dev)
>
>         /* Prevent recursive tx locking */
>         dev->features |= NETIF_F_LLTX;
> -       /* VLAN on top of HSR needs testing and probably some work on
> -        * hsr_header_create() etc.
> -        */
> -       dev->features |= NETIF_F_VLAN_CHALLENGED;
>         /* Not sure about this. Taken from bridge code. netdev_features.h says
>          * it means "Does not change network namespaces".
>          */
> diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
> index cadfccd7876e..de21df30b0d9 100644
> --- a/net/hsr/hsr_forward.c
> +++ b/net/hsr/hsr_forward.c
> @@ -208,6 +208,7 @@ static struct sk_buff *hsr_fill_tag(struct sk_buff *skb,
>                                     struct hsr_port *port, u8 proto_version)
>  {
>         struct hsr_ethhdr *hsr_ethhdr;
> +       unsigned char *pc;
>         int lsdu_size;
>
>         /* pad to minimum packet size which is 60 + 6 (HSR tag) */
> @@ -218,7 +219,18 @@ static struct sk_buff *hsr_fill_tag(struct sk_buff *skb,
>         if (frame->is_vlan)
>                 lsdu_size -= 4;
>
> -       hsr_ethhdr = (struct hsr_ethhdr *)skb_mac_header(skb);
> +       pc = skb_mac_header(skb);
> +       if (frame->is_vlan)
> +               /* This 4-byte shift (size of a vlan tag) does not
> +                * mean that the ethhdr starts there. But rather it
> +                * provides the proper environment for accessing
> +                * the fields, such as hsr_tag etc., just like
> +                * when the vlan tag is not there. This is because
> +                * the hsr tag is after the vlan tag.
> +                */
> +               hsr_ethhdr = (struct hsr_ethhdr *)(pc + VLAN_HLEN);
> +       else
> +               hsr_ethhdr = (struct hsr_ethhdr *)pc;

Instead, I would pass the header from the caller, which knows the
offset because it moves the previous headers to make space.

Also, supporting VLAN probably also requires supporting 802.1ad QinQ,
which means code should parse the headers instead of hardcoding
VLAN_HLEN.

>         hsr_set_path_id(hsr_ethhdr, port);
>         set_hsr_tag_LSDU_size(&hsr_ethhdr->hsr_tag, lsdu_size);
> @@ -511,8 +523,6 @@ static int fill_frame_info(struct hsr_frame_info *frame,
>         if (frame->is_vlan) {
>                 vlan_hdr = (struct hsr_vlan_ethhdr *)ethhdr;
>                 proto = vlan_hdr->vlanhdr.h_vlan_encapsulated_proto;
> -               /* FIXME: */
> -               netdev_warn_once(skb->dev, "VLAN not yet supported");
>         }
>
>         frame->is_from_san = false;
> --
> 2.17.1
>

Powered by blists - more mailing lists