Re: [PATCH] net: ipv6: Add support for default gateway discovery.

Vyacheslav V. Mitrofanov
2023-03-23T16:59:14Z

On Thu, 2023-03-23 at 16:44 +0000, Ehsan Mohandesi wrote:
>
> Hi Viacheslav,
>
> > -----Original Message-----
> > From: Vyacheslav V. Mitrofanov <v.v.mitrofanov@yadro.com>
> > Sent: Thursday, March 16, 2023 3:47 AM
> > To: u-boot@lists.denx.de; emohandesi@linux.microsoft.com
> > Cc: joe.hershberger@ni.com; xypron.glpk@gmx.de;
> > dphadke@linux.microsoft.com; saproj@gmail.com; rfried.dev@gmail.com
> > ;
> > ilias.apalodimas@linaro.org; Ehsan Mohandesi <
> > emohandesi@microsoft.com>;
> > john@metanate.com; sjg@chromium.org; masahisa.kojima@linaro.org
> > Subject: [EXTERNAL] Re: [PATCH] net: ipv6: Add support for default
> > gateway
> > discovery.
> >
> > On Thu, 2023-03-02 at 08:58 -0800, emohandesi@linux.microsoft.com
> > wrote:
> > > From: Ehsan Mohandesi <emohandesi@microsoft.com>
> > >
> > > In IPv6, the default gateway and prefix length are determined by
> > > receiving a router advertisement as defined in -
> > >
> > https://www.rf/
> > c-
> > editor.org%2Frfc%2Frfc4861&data=05%7C01%7Cemohandesi%40microsoft.co
> > m%7C6dec635abc8c4861feb708db25fb05d6%7C72f988bf86f141af91ab2d7cd01
> > 1db47%7C1%7C0%7C638145532341238481%7CUnknown%7CTWFpbGZsb3d8ey
> > JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C
> > 3000%7C%7C%7C&sdata=tAhREBvBgVQKOFqEQT2%2FKphGxYXUMo3UF5vvQpY
> > B%2Be0%3D&reserved=0.
> > > Add support for sending router solicitation (RS) and processing
> > > router
> > > advertisements (RA).
> > >
> > > If the RA has prefix info option and following conditions are
> > > met,
> > > then
> > > gatewayip6 and net_prefix_length of ip6addr env variables are
> > > initialized.
> > > These are later consumed by IPv6 code for non-local destination
> > > IP.
> > >
> > > - "Router Lifetime" != 0
> > > - Prefix is NOT link-local prefix (0xfe80::/10)
> > > - L flag is 1
> > > - "Valid Lifetime" != 0
> > >
> > > Timing Parameters:
> > > - MAX_RTR_SOLICITATION_DELAY (0-1s)
> > > - RTR_SOLICITATION_INTERVAL (4s) (min retransmit delay)
> > > - MAX_RTR_SOLICITATIONS (3 RS transmissions)
> > >
> > > The functionality is enabled by CONFIG_IPV6_ROUTER_DISCOVERY and
> > > invoked automatically from net_init_loop().
> > >
> > > Signed-off-by: Ehsan Mohandesi <emohandesi@microsoft.com>
> > >
> > > Conflicts:
> > > cmd/Kconfig
> > > include/net.h
> > > net/net.c
> > > ---
> > > cmd/Kconfig | 7 ++
> > > include/ndisc.h | 23 ++++++
> > > include/net.h | 2 +-
> > > include/net6.h | 40 ++++++++++
> > > net/ndisc.c | 243
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++---
> > > net/net.c | 23 +++++-
> > > net/net6.c | 1 +
> > > 7 files changed, 327 insertions(+), 12 deletions(-)
> > >
> >
> > I reviewed this patch and it looks good. I have no critical
> > remarks, only some
> > small notes.
> >
> > I've tested it on SiFive Unmatched board.
> >
> >
> > > +config IPV6_ROUTER_DISCOVERY
> > > + bool "Do router discovery"
> > > + depends on IPV6
> > > + help
> > > + Will automatically perform router solicitation on first
> > > IPv6
> > > + network operation
> > > +
> > > endif
> > >
> > I think it is better to write sth like Do IPv6 router discovery
> > because
> > IPv4 has also router discovery protocol and it could lead to
> > misunderstanding
> >
> >
> > > net_set_timeout_handler(0, 0);
> > >
> > Maybe net_set_timeout_handler(0, NULL); is better
> >
> >
> >
> > > +/*
> > > + * validate_ra() - Validate the router advertisement message.
> > > + *
> > > + * @ip6:
> > > + * @len: Length of the router advertisement packet
> > > + *
> > > + * Check if the router advertisement message is valid.
> > > Conditions
> > > are
> > > + * according to RFC 4861 section 6.1.2. Validation of Router
> > > Advertisement
> > > + * Messages.
> > > + *
> > > + * Return: true if the message is valid and false if it is
> > > invalid.
> > > + */
> > > +static bool validate_ra(struct ip6_hdr *ip6, int len) {
> > > + struct icmp6hdr *icmp = (struct icmp6hdr *)(ip6 + 1);
> > > +
> > > + /* ICMP length (derived from the IP length) should be 16
> > > or
> > > more octets. */
> > > + if (ip6->payload_len < 16)
> > > + return false;
> > > +
> > > + /* Source IP Address should be a valid link-local
> > > address. */
> > > + if ((ntohs(ip6->saddr.s6_addr16[0]) &
> > > IPV6_LINK_LOCAL_MASK)
> > > !=
> > > + IPV6_LINK_LOCAL_PREFIX)
> > > + return false;
> > > +
> > > + /*
> > > + * The IP Hop Limit field should have a value of 255,
> > > i.e.,
> > > the packet
> > > + * could not possibly have been forwarded by a router.
> > > + */
> > > + if (ip6->hop_limit != 255)
> > > + return false;
> > > +
> > Unicast hop limit only?
>
> Sorry, I do not understand what you mean here. What kind of scenario
> are you talking about?
> It does not matter if the router advertisement is unicast or
> multicast. In both cases, the hop limit needs to be 255. A router
> always sets the hop limit to 255. We do not want an advertisement
> that is forwarded from another node. Refer to this for more
> information.
> https://www.rfc-editor.org/rfc/rfc4861#section-6.1.2
>

Yes, I got you. I have no objections.
Thanks!