[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZLqUfwKkRH85uPlT@smile.fi.intel.com>
Date: Fri, 21 Jul 2023 17:21:51 +0300
From: Andy Shevchenko <andy@...nel.org>
To: Marcin Szycik <marcin.szycik@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
wojciech.drewek@...el.com, michal.swiatkowski@...ux.intel.com,
aleksander.lobakin@...el.com, davem@...emloft.net, kuba@...nel.org,
jiri@...nulli.us, pabeni@...hat.com, jesse.brandeburg@...el.com,
simon.horman@...igine.com, idosch@...dia.com
Subject: Re: [PATCH iwl-next v3 1/6] ip_tunnel: use a separate struct to
store tunnel params in the kernel
On Fri, Jul 21, 2023 at 09:15:27AM +0200, Marcin Szycik wrote:
> From: Alexander Lobakin <aleksander.lobakin@...el.com>
>
> Unlike IPv6 tunnels which use purely-kernel __ip6_tnl_parm structure
> to store params inside the kernel, IPv4 tunnel code uses the same
> ip_tunnel_parm which is being used to talk with the userspace.
> This makes it difficult to alter or add any fields or use a
> different format for whatever data.
> Define struct ip_tunnel_parm_kern, a 1:1 copy of ip_tunnel_parm for
> now, and use it throughout the code. The two places where the latter
> is used to interact with the userspace, now do a conversion from one
> type to another, with manual field-by-field assignments.
> Must be done at once, since ip_tunnel::parms is being used in most
> of those places.
...
> + strscpy(kp.name, p.name, sizeof(kp.name));
> + kp.link = p.link;
> + kp.i_flags = p.i_flags;
> + kp.o_flags = p.o_flags;
> + kp.i_key = p.i_key;
> + kp.o_key = p.o_key;
> + memcpy(&kp.iph, &p.iph, min(sizeof(kp.iph), sizeof(p.iph)));
> +
> + err = dev->netdev_ops->ndo_tunnel_ctl(dev, &kp, cmd);
> + if (err)
> + return err;
> +
> + strscpy(p.name, kp.name, sizeof(p.name));
> + p.link = kp.link;
> + p.i_flags = kp.i_flags;
> + p.o_flags = kp.o_flags;
> + p.i_key = kp.i_key;
> + p.o_key = kp.o_key;
> + memcpy(&p.iph, &kp.iph, min(sizeof(p.iph), sizeof(kp.iph)));
> + strscpy(kp.name, p.name, sizeof(kp.name));
> + kp.link = p.link;
> + kp.i_flags = p.i_flags;
> + kp.o_flags = p.o_flags;
> + kp.i_key = p.i_key;
> + kp.o_key = p.o_key;
> + memcpy(&kp.iph, &p.iph, min(sizeof(p.iph), sizeof(kp.iph)));
Seems to me these two deserves separate helpers to avoid such a duplication(s).
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists