Re: [PATCH] rtnetlink.7: Document struct ifa_cacheinfo

Alex Henrie
2024-11-11T06:16:50Z

On Tue, Nov 5, 2024 at 4:33 AM Alejandro Colomar <alx@kernel.org> wrote:

> On Mon, Nov 04, 2024 at 09:53:38PM GMT, Kuniyuki Iwashima wrote:
> > From: Alex Henrie <alexhenrie24@gmail.com>
> > Date: Mon, 4 Nov 2024 21:14:20 -0700
> > > struct ifa_cacheinfo contains the address's creation time, update time,
> > > preferred lifetime, and valid lifetime. See
>
> We use two spaces after period (the correct amount). :)
> (I'm thinking we probably want to document something about it in
> man-pages(7). Branden, do you want to send a patch about it? I want
> to include the references you showed to me, and you probably remember
> better those links.)
>
> > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/if_addr.h?h=v6.11#n60
>
> Please use this format for links:
>
> Link: <http://example.com>

Since the second sentence will be eliminated in favor of a Link line,
the first sentence will no longer have any spaces after its period.

> Which include provides the structure?

linux/if_addr.h, which is the file I linked to in the commit message,
and the same file that contains struct ifaddrmsg which is documented a
few paragraphs earlier in the same section of the man page.

> > > +struct ifa_cacheinfo {
> > > + __u32 ifa_prefered; /* Preferred lifetime in seconds, -1 = forever */
> > > + __u32 ifa_valid; /* Valid lifetime in seconds, -1 = forever */
> >
> > -1 should be rather 0xFFFFFFFF (INFINITY_LIFE_TIME) as it's unsigned.
>
> I prefer UINT32_MAX over 0xF...F, which might be unclear how many Fs it
> has.

INFINITY_LIFE_TIME is not defined in any public header, so let's not
mention it. I agree that it's hard to see at a glance how many F's are
in 0xFFFFFFFF. I would suggest ~0u, which is short and sweet, but
UINT32_MAX is a little better because ~0u isn't 32 bits on all C
compilers that have ever existed.

> > Also, it would be nice to mention that ifa_prefered must be less than
> > or equal to ifa_valid (ifa_prefered <= ifa_valid) and 0 is invalid for
> > ifa_valid.
> >
> > 0 <= ifa_prefered <= ifa_valid
> > 0 < ifa_valid <= 0xFFFFFFFF

I'll add a paragraph to explain those relationships.

> It might also be interesting to add a separate manual page for the type,
> and reference it here. Otherwise, the page starts getting fatty.

Perhaps. In my opinion, there's not enough material here to be worthy
of its own page.

Thanks for the feedback,

-Alex