netdev - Re: [PATCH net 5/5] nfp: remove false positive offloads in flower vxlan

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]

Message-ID: <CAJ3xEMgmUFS5vEzy-sRZ7XzFsHVSaKKLDuBoba0WZvauP9ZmLw@mail.gmail.com>
Date:   Wed, 18 Apr 2018 10:43:45 +0300
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     Linux Netdev List <netdev@...r.kernel.org>,
        oss-drivers@...ronome.com, John Hurley <john.hurley@...ronome.com>
Subject: Re: [PATCH net 5/5] nfp: remove false positive offloads in flower vxlan

On Fri, Nov 17, 2017 at 4:06 AM, Jakub Kicinski
<jakub.kicinski@...ronome.com> wrote:
> From: John Hurley <john.hurley@...ronome.com>
>
> Pass information to the match offload on whether or not the repr is the
> ingress or egress dev. Only accept tunnel matches if repr is the egress dev.
>
> This means rules such as the following are successfully offloaded:
> tc .. add dev vxlan0 .. enc_dst_port 4789 .. action redirect dev nfp_p0
>
> While rules such as the following are rejected:
> tc .. add dev nfp_p0 .. enc_dst_port 4789 .. action redirect dev vxlan0

cool


> Also reject non tunnel flows that are offloaded to an egress dev.
> Non tunnel matches assume that the offload dev is the ingress port and
> offload a match accordingly.

not following on the "Also" here, see below


> diff --git a/drivers/net/ethernet/netronome/nfp/flower/offload.c b/drivers/net/ethernet/netronome/nfp/flower/offload.c
> index a0193e0c24a0..f5d73b83dcc2 100644
> --- a/drivers/net/ethernet/netronome/nfp/flower/offload.c
> +++ b/drivers/net/ethernet/netronome/nfp/flower/offload.c
> @@ -131,7 +131,8 @@ static bool nfp_flower_check_higher_than_mac(struct tc_cls_flower_offload *f)
>
>  static int
>  nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
> -                               struct tc_cls_flower_offload *flow)
> +                               struct tc_cls_flower_offload *flow,
> +                               bool egress)
>  {
>         struct flow_dissector_key_basic *mask_basic = NULL;
>         struct flow_dissector_key_basic *key_basic = NULL;
> @@ -167,6 +168,9 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
>                         skb_flow_dissector_target(flow->dissector,
>                                                   FLOW_DISSECTOR_KEY_ENC_CONTROL,
>                                                   flow->key);
> +               if (!egress)
> +                       return -EOPNOTSUPP;
> +
>                 if (mask_enc_ctl->addr_type != 0xffff ||
>                     enc_ctl->addr_type != FLOW_DISSECTOR_KEY_IPV4_ADDRS)
>                         return -EOPNOTSUPP;
> @@ -194,6 +198,9 @@ nfp_flower_calculate_key_layers(struct nfp_fl_key_ls *ret_key_ls,
>
>                 key_layer |= NFP_FLOWER_LAYER_VXLAN;
>                 key_size += sizeof(struct nfp_flower_vxlan);
> +       } else if (egress) {
> +               /* Reject non tunnel matches offloaded to egress repr. */
> +               return -EOPNOTSUPP;
>         }

with these two hunks we get: egress <- IFF -> encap match, right?

(1) we can't offload the egress way if there isn't matching on encap headers
(2) we can't go the matching on encap headers way if we are not egress

what other cases are rejected by this logic?

e.g If we add a rule with SW device (veth. tap) being the ingress, and
HW device (vf rep)
being the egress while not using skip_sw (just no flags == both) we
get the TC stack
go along the egdev callback from the vf rep hw device and add an
(uplink --> vf rep) rule
which will not be rejected if there is matching on tunnel headers, it
will also not be rejected
by some driver logic as the one we discussed to identify and ignore
rules that are attempted to being added twice.

Or.