linux-kernel - Re: [PATCH net-next V5] net: Variable SLAAC: SLAAC with prefixes of arbitrary length in PIO

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: <20201118075027.18083d19@kicinski-fedora-PC1C0HJN.hsd1.ca.comcast.net>
Date:   Wed, 18 Nov 2020 07:50:27 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Dmytro Shytyi <dmytro@...tyi.net>
Cc:     "kuznet" <kuznet@....inr.ac.ru>,
        "yoshfuji" <yoshfuji@...ux-ipv6.org>,
        "liuhangbin" <liuhangbin@...il.com>, "davem" <davem@...emloft.net>,
        "netdev" <netdev@...r.kernel.org>,
        "linux-kernel" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next V5] net: Variable SLAAC: SLAAC with prefixes of
 arbitrary length in PIO

On Wed, 18 Nov 2020 14:41:03 +0100 Dmytro Shytyi wrote:
>  > > @@ -2576,9 +2590,42 @@ int addrconf_prefix_rcv_add_addr(struct 
>  > >                   u32 addr_flags, bool sllao, bool tokenized, 
>  > >                   __u32 valid_lft, u32 prefered_lft) 
>  > >  { 
>  > > -    struct inet6_ifaddr *ifp = ipv6_get_ifaddr(net, addr, dev, 1); 
>  > > +    struct inet6_ifaddr *ifp = NULL; 
>  > >      int create = 0; 
>  > > 
>  > > +    if ((in6_dev->if_flags & IF_RA_VAR_PLEN) == IF_RA_VAR_PLEN && 
>  > > +        in6_dev->cnf.addr_gen_mode != IN6_ADDR_GEN_MODE_STABLE_PRIVACY) { 
>  > > +        struct inet6_ifaddr *result = NULL; 
>  > > +        struct inet6_ifaddr *result_base = NULL; 
>  > > +        struct in6_addr curr_net_prfx; 
>  > > +        struct in6_addr net_prfx; 
>  > > +        bool prfxs_equal; 
>  > > + 
>  > > +        result_base = result; 
>  > > +        rcu_read_lock(); 
>  > > +        list_for_each_entry_rcu(ifp, &in6_dev->addr_list, if_list) { 
>  > > +            if (!net_eq(dev_net(ifp->idev->dev), net)) 
>  > > +                continue; 
>  > > +            ipv6_addr_prefix_copy(&net_prfx, &pinfo->prefix, pinfo->prefix_len); 
>  > > +            ipv6_addr_prefix_copy(&curr_net_prfx, &ifp->addr, pinfo->prefix_len); 
>  > > +            prfxs_equal = 
>  > > +                ipv6_prefix_equal(&net_prfx, &curr_net_prfx, pinfo->prefix_len); 
>  > > + 
>  > > +            if (prfxs_equal && pinfo->prefix_len == ifp->prefix_len) { 
>  > > +                result = ifp; 
>  > > +                in6_ifa_hold(ifp); 
>  > > +                break; 
>  > > +            } 
>  > > +        } 
>  > > +        rcu_read_unlock(); 
>  > > +        if (result_base != result) 
>  > > +            ifp = result; 
>  > > +        else 
>  > > +            ifp = NULL;   
>  >  
>  > Could this be a helper of its own?   
> 
> Explain the thought please. It is not clear for me.

At the look of it the code under this if statement looks relatively
self-contained, and has quite a few local variables. Rather than making
the surrounding function longer would it be possible to factor it out
into a helper function?