Re: [PATCH net-next 2/4] netconsole: use the address family instead of the ipv6 flag

On Thu, Aug 6, 2026 at 4:55 PM Breno Leitao <[email protected]> wrote:
>
> On Wed, Aug 05, 2026 at 10:33:02PM +0100, Gustavo Luiz Duarte wrote:
> > Now that we have the address family in inet_addr, use that and remove
> > nt->ipv6.
> >
> > We no longer need netcons_local_ip_unset() to check that all bytes are
> > zeroes, as that is now denoted by (family == AF_UNSPEC).
> >
> > Signed-off-by: Gustavo Luiz Duarte <[email protected]>
> > ---
> >  drivers/net/netconsole.c | 82 ++++++++++++++----------------------------------
> >  1 file changed, 23 insertions(+), 59 deletions(-)
> >
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index d615a9256787..070bae7b4fd7 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -181,7 +181,6 @@ enum target_state {
> >   *           local_mac       (read-only)
> >   * @local_ip:        Source IP address of the target (read-write).
> >   * @remote_ip:       Destination IP address of the target (read-write).
> > - * @ipv6:    Whether the target addresses are IPv6 (read-write).
> >   * @local_port:      Source UDP port of the target (read-write).
> >   * @remote_port: Destination UDP port of the target (read-write).
> >   * @remote_mac:      Destination ethernet address of the target (read-write).
> > @@ -213,7 +212,6 @@ struct netconsole_target {
> >       bool                    release;
> >       struct netpoll          np;
> >       struct inet_addr        local_ip, remote_ip;
> > -     bool                    ipv6;
> >       u16                     local_port, remote_port;
> >       u8                      remote_mac[ETH_ALEN];
> >       /* protected by target_list_lock; +1 gives scnprintf() room for its
> > @@ -463,23 +461,6 @@ static int netcons_take_ipv4(struct netconsole_target *nt,
> >       return 0;
> >  }
> >
> > -/*
> > - * Test whether the caller left nt->local_ip unset, so that
> > - * netcons_netpoll_setup() should auto-populate it from the egress device.
> > - *
> > - * nt->local_ip is a union of __be32 (IPv4) and struct in6_addr (IPv6),
> > - * so an IPv6 address whose first 4 bytes are zero (e.g. ::1, ::2,
> > - * IPv4-mapped ::ffff:a.b.c.d) must not be tested via the IPv4 arm —
> > - * doing so would misclassify a caller-supplied address as unset and
> > - * silently overwrite it with whatever address the device exposes.
> > - */
> > -static bool netcons_local_ip_unset(const struct netconsole_target *nt)
> > -{
> > -     if (nt->ipv6)
> > -             return ipv6_addr_any(&nt->local_ip.in6);
> > -     return !nt->local_ip.ip;
> > -}
> > -
> >  static int netcons_netpoll_setup(struct netconsole_target *nt)
> >  {
> >       struct net *net = current->nsproxy->net_ns;
> > @@ -525,16 +506,13 @@ static int netcons_netpoll_setup(struct netconsole_target *nt)
> >               rtnl_lock();
> >       }
> >
> > -     if (netcons_local_ip_unset(nt)) {
> > -             if (!nt->ipv6) {
> > -                     err = netcons_take_ipv4(nt, ndev);
> > -                     if (err)
> > -                             goto put;
> > -             } else {
> > +     if (nt->local_ip.family == AF_UNSPEC) {
> > +             if (nt->remote_ip.family == AF_INET6)
> >                       err = netcons_take_ipv6(nt, ndev);
> > -                     if (err)
> > -                             goto put;
> > -             }
> > +             else
> > +                     err = netcons_take_ipv4(nt, ndev);
> > +             if (err)
> > +                     goto put;
>
> nice, that is much clearer.
>
> >               ip_overwritten = true;
> >       }
> >
> > @@ -716,22 +694,22 @@ static void netconsole_print_banner(struct netconsole_target *nt)
> >       struct netpoll *np = &nt->np;
> >
> >       np_info(np, "local port %d\n", nt->local_port);
> > -     if (nt->ipv6)
> > +     if (nt->local_ip.family == AF_INET6)
> >               np_info(np, "local IPv6 address %pI6c\n", &nt->local_ip.in6);
> >       else
> >               np_info(np, "local IPv4 address %pI4\n", &nt->local_ip.ip);
>
> Any chance it can be UNSPEC?

Yes. Since this is a refactoring patch, I didn't want to change behavior here.
So I kept the original behavior which is print "0.0.0.0".

I was planning to send a separate patch set to change this:
a) Print an empty string if unset (AF_UNSPEC).
b) Allow the user to write an empty string to local_ip or remote_ip to
denote unset.

This can be useful in case the user is reconfiguring the target and
wants to make use of the local_ip automatic discovery logic from
netconsole.

>
> >       np_info(np, "interface name '%s'\n", np->dev_name);
> >       np_info(np, "local ethernet address '%pM'\n", np->dev_mac);
> >       np_info(np, "remote port %d\n", nt->remote_port);
> > -     if (nt->ipv6)
> > +     if (nt->remote_ip.family == AF_INET6)
> >               np_info(np, "remote IPv6 address %pI6c\n", &nt->remote_ip.in6);
> >       else
>
> Same question
>
> > @@ -755,7 +733,7 @@ static int netpoll_parse_ip_addr(const char *str, struct inet_addr *addr)
> >           in6_pton(str, len, (void *)&addr->in6, -1, &end) > 0 &&
> >           (!end || *end == 0 || *end == '\n')) {
> >               addr->family = AF_INET6;
>
> Why not setting family for for IPV4? In
>
>         if (in4_pton(str, len, (void *)addr, -1, &end) > 0 &&
>             (!end || *end == 0 || *end == '\n'))
>                 return 0;

I did that in the previous patch. Here we are just changing the return code
convention for netpoll_parse_ip_addr(). We no longer need to return the address
family because it is now baked into inet_address. So instead of returning 0 for
ipv4, 1 for ipv6, and -1 for failure, we just return -1 for failure and 0 for
success.