netdev - Re: [PATCH] net: mlx5: allow default ip

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] [day] [month] [year] [list]

Date:   Thu, 24 Jan 2019 19:14:32 +0200
From:   Or Gerlitz <gerlitz.or@...il.com>
To:     Tonghao Zhang <xiangxia.m.yue@...il.com>
Cc:     Or Gerlitz <ogerlitz@...lanox.com>,
        Saeed Mahameed <saeedm@...lanox.com>,
        Leon Romanovsky <leonro@...lanox.com>,
        Linux Netdev List <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: mlx5: allow default ip_proto to offload

On Sat, Jan 19, 2019 at 10:50 AM Tonghao Zhang <xiangxia.m.yue@...il.com> wrote:
> On Fri, Jan 18, 2019 at 10:19 PM Or Gerlitz <gerlitz.or@...il.com> wrote:
> > On Thu, Jan 17, 2019 at 3:34 PM Tonghao Zhang <xiangxia.m.yue@...il.com> wrote:
> > > On Thu, Jan 17, 2019 at 8:58 PM Or Gerlitz <gerlitz.or@...il.com> wrote:
> > > > On Thu, Jan 17, 2019 at 11:28 AM <xiangxia.m.yue@...il.com> wrote:
> > > > > From: Tonghao Zhang <xiangxia.m.yue@...il.com>
> >
> > > with this patch, run the command [2], we will not get err log,
> > > and the filter work in hw.
> >
> > This whole thing is done for a reason which is the inability of the current HW
> > to adjust checksum/crc for few L3 protocols. Such adjustment is needed if
> > you modify some fields of L3 headers, e.g re-write src/dst IP address.
> I got it, thanks
> > > We should consider ip_proto == 0, in some case, we only
> > > modify dest ip or src ip.
> >
> > we can't let it go without clear matching on the ip protocol, as I explained
> > above. With my proposed patch you will be able to NAT much more protocols
> > (all of them expect for three, and we're working to reduce that), but
> > you still need
> > a tc rule per ip proto
>
>
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index 608025ca5c04..affb523e0e35 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -2167,11 +2167,11 @@ static bool
> > modify_header_match_supported(struct mlx5_flow_spec *spec,
> >         }
> >
> >         ip_proto = MLX5_GET(fte_match_set_lyr_2_4, headers_v, ip_protocol);
> > -       if (modify_ip_header && ip_proto != IPPROTO_TCP &&
> > -           ip_proto != IPPROTO_UDP && ip_proto != IPPROTO_ICMP) {
> > +       if (modify_ip_header && (ip_proto == IPPROTO_ICMPV6 ||
> > +           ip_proto == IPPROTO_SCTP || ip_proto == IPPROTO_UDPLITE)) {
> >                 NL_SET_ERR_MSG_MOD(extack,
> > -                                  "can't offload re-write of non TCP/UDP");
> > -               pr_info("can't offload re-write of ip proto %d\n", ip_proto);
> > +                                  "can't offload this re-write of IP
> > addresses");
> > +               pr_info("can't offload re-write of IP addrs for ip
> > proto %d\n", ip_proto);
> >                 return false;
> >         }

> This patch work for me too, because ip_proto == 0 will not return err(
> and my patch allow ip_proto == 0 and not return err) and will you send
> it to net-next ? because i can't find it in net-next.

basically, I was planning to upstream it on this cycle, but your
comment below is
something I need to look at

> and one question, In your patch, should we check ip_proto is valid ?
> for example, ip_proto == 18,  is not valid protocol.

yeah, this becomes a bit ugly, I need to see how to address that

> flower ip_proto 18