On Sat, Jan 19, 2019 at 10:50 AM Tonghao Zhang <xiangxia.m....@gmail.com> wrote:
> On Fri, Jan 18, 2019 at 10:19 PM Or Gerlitz <gerlitz...@gmail.com> wrote:
> > On Thu, Jan 17, 2019 at 3:34 PM Tonghao Zhang <xiangxia.m....@gmail.com>
> > wrote:
> > > On Thu, Jan 17, 2019 at 8:58 PM Or Gerlitz <gerlitz...@gmail.com> wrote:
> > > > On Thu, Jan 17, 2019 at 11:28 AM <xiangxia.m....@gmail.com> wrote:
> > > > > From: Tonghao Zhang <xiangxia.m....@gmail.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
