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]
Message-ID: <CABAhCOQy-qw8pY+8XjHGPVz7jWZ7wqnadPXZrF-enAO0AEgXyQ@mail.gmail.com>
Date: Tue, 14 Jan 2025 17:02:40 +0800
From: Xiao Liang <shaw.leon@...il.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>
Cc: alex.aring@...il.com, andrew+netdev@...n.ch, 
	b.a.t.m.a.n@...ts.open-mesh.org, bpf@...r.kernel.org, bridge@...ts.linux.dev, 
	davem@...emloft.net, donald.hunter@...il.com, dsahern@...nel.org, 
	edumazet@...gle.com, herbert@...dor.apana.org.au, horms@...nel.org, 
	kuba@...nel.org, linux-can@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kselftest@...r.kernel.org, linux-ppp@...r.kernel.org, 
	linux-rdma@...r.kernel.org, linux-wireless@...r.kernel.org, 
	linux-wpan@...r.kernel.org, miquel.raynal@...tlin.com, netdev@...r.kernel.org, 
	osmocom-net-gprs@...ts.osmocom.org, pabeni@...hat.com, shuah@...nel.org, 
	stefan@...enfreihafen.org, steffen.klassert@...unet.com, 
	wireguard@...ts.zx2c4.com
Subject: Re: [PATCH net-next v8 06/11] net: ipv6: Use link netns in newlink()
 of rtnl_link_ops

On Tue, Jan 14, 2025 at 12:49 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
>
> From: Xiao Liang <shaw.leon@...il.com>
> Date: Mon, 13 Jan 2025 22:37:14 +0800
> > diff --git a/drivers/net/bonding/bond_netlink.c b/drivers/net/bonding/bond_netlink.c
> > index 2a6a424806aa..ac5e402c34bc 100644
> > --- a/drivers/net/bonding/bond_netlink.c
> > +++ b/drivers/net/bonding/bond_netlink.c
> > @@ -564,10 +564,12 @@ static int bond_changelink(struct net_device *bond_dev, struct nlattr *tb[],
> >       return 0;
> >  }
> >
> > -static int bond_newlink(struct net *src_net, struct net_device *bond_dev,
> > -                     struct nlattr *tb[], struct nlattr *data[],
> > +static int bond_newlink(struct net_device *bond_dev,
> > +                     struct rtnl_newlink_params *params,
> >                       struct netlink_ext_ack *extack)
> >  {
> > +     struct nlattr **data = params->data;
> > +     struct nlattr **tb = params->tb;
> >       int err;
> >
> >       err = bond_changelink(bond_dev, tb, data, extack);
>
> Note that IFLA_BOND_ACTIVE_SLAVE uses dev_net(dev) for
> __dev_get_by_index().

That's true. Bond devices have no "link-netns", and a slave
device must be in the same namespace of the main dev.

> [...]
> > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > index fed4fe2a4748..0c496aa1f706 100644
> > --- a/drivers/net/macvlan.c
> > +++ b/drivers/net/macvlan.c
> > @@ -1565,11 +1565,12 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> >  }
> >  EXPORT_SYMBOL_GPL(macvlan_common_newlink);
> >
> > -static int macvlan_newlink(struct net *src_net, struct net_device *dev,
> > -                        struct nlattr *tb[], struct nlattr *data[],
> > +static int macvlan_newlink(struct net_device *dev,
> > +                        struct rtnl_newlink_params *params,
> >                          struct netlink_ext_ack *extack)
> >  {
> > -     return macvlan_common_newlink(src_net, dev, tb, data, extack);
> > +     return macvlan_common_newlink(params->net, dev, params->tb,
> > +                                   params->data, extack);
>
> Pass params as is as you did for ipvlan_link_new().
>
> Same for macvtap_newlink().

OK.

> [...]
> > diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
> > index 1e1b00756be7..1e9eadc77da2 100644
> > --- a/drivers/net/netkit.c
> > +++ b/drivers/net/netkit.c
> > @@ -327,10 +327,13 @@ static int netkit_validate(struct nlattr *tb[], struct nlattr *data[],
> >
> >  static struct rtnl_link_ops netkit_link_ops;
> >
> > -static int netkit_new_link(struct net *peer_net, struct net_device *dev,
> > -                        struct nlattr *tb[], struct nlattr *data[],
> > +static int netkit_new_link(struct net_device *dev,
> > +                        struct rtnl_newlink_params *params,
> >                          struct netlink_ext_ack *extack)
> >  {
> > +     struct nlattr **data = params->data;
> > +     struct net *peer_net = params->net;
> > +     struct nlattr **tb = params->tb;
>
> nit: please keep the reverse xmas tree order.
>
>
> >       struct nlattr *peer_tb[IFLA_MAX + 1], **tbp = tb, *attr;
>
> you can define *tbp here and initialise it later.
>
>         struct nlattr *peer_tb[IFLA_MAX + 1], **tbp, *attr;
>
> >       enum netkit_action policy_prim = NETKIT_PASS;
> >       enum netkit_action policy_peer = NETKIT_PASS;
>
>
> [...]
> > @@ -1064,6 +1067,11 @@ static void wwan_create_default_link(struct wwan_device *wwandev,
> >       struct net_device *dev;
> >       struct nlmsghdr *nlh;
> >       struct sk_buff *msg;
> > +     struct rtnl_newlink_params params = {
> > +             .net = &init_net,
> > +             .tb = tb,
> > +             .data = data,
> > +     };
>
> nit: Reverse xmas tree order
>
>
> [...]
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index ec98349b9620..7ff5e96f6ba7 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -3766,6 +3766,14 @@ static int rtnl_newlink_create(struct sk_buff *skb, struct ifinfomsg *ifm,
> >       struct net_device *dev;
> >       char ifname[IFNAMSIZ];
> >       int err;
> > +     struct rtnl_newlink_params params = {
>
> nit: Reverse xmas tree order
>
>
> > +             .net = net,
>
> Use sock_net(skb->sk) directly here and remove net defined above,
> which is no longer used in this function.
>
> ---8<---
>         unsigned char name_assign_type = NET_NAME_USER;
>         struct rtnl_newlink_params params = {
>                 .net = sock_net(skb->sk),
>                 .src_net = net,
>                 .link_net = link_net,
>                 .peer_net = peer_net,
>                 .tb = tb,
>                 .data = data,
>         };
>         u32 portid = NETLINK_CB(skb).portid;
> ---8<---
>
>
> [...]
> > @@ -1698,6 +1702,10 @@ struct net_device *gretap_fb_dev_create(struct net *net, const char *name,
> >       LIST_HEAD(list_kill);
> >       struct ip_tunnel *t;
> >       int err;
> > +     struct rtnl_newlink_params params = {
> > +             .net = net,
> > +             .tb = tb,
> > +     };
> >
> >       memset(&tb, 0, sizeof(tb));
>
> nit: Reverse xmas tree

Will fix the style issues mentioned above in the next version.

Thanks.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ