Subject: Re: [PATCH 1/2] NFS: change the ip_map cache code to handle IPv6 addresses

From: Brian Haley <[email-masked]>
Date: Fri, 12 Oct 2007 12:07:46 -0400

Hi Aurelien,

There were some of my comments you haven't addressed yet, comments in-line.

Aurélien Charbon wrote:
> Here is a patch for the ip_map caching code part in nfs server.

> +    for (i = 0; i < ncp->cl_naddr; i++) {
> +        /* Mapping address */
> +        ipv6_addr_v4map(ncp->cl_addrlist[i], addr6);

ipv6_addr_set(&addr6, 0, 0, htonl(0x0000FFFF), ncp->cl_addrlist[i]);

> +    /* IPv6 address mapping */
> +    ipv6_addr_v4map(sin->sin_addr, in6);

ipv6_addr_set(&in6, 0, 0, htonl(0x0000FFFF), sin->sin_addr);

> +    /* IPv6 address mapping */
> +    ipv6_addr_v4map(sin->sin_addr, in6);

ipv6_addr_set(&in6, 0, 0, htonl(0x0000FFFF), sin->sin_addr);

> +#define IS_ADDR_MAPPED(a) \
> +    (((uint32_t *) (a))[0] == 0            \
> +    && ((uint32_t *) (a))[1] == 0            \
> +    && (((uint32_t *) (a))[2] == 0            \
> +    || ((uint32_t *) (a))[2] == htonl(0xffff)))

This is unused, can go away.

> +static inline void ipv6_addr_v4map(const struct in_addr a1, struct 
> in6_addr a2)
> +{
> +    a2.s6_addr32[0] = 0;
> +    a2.s6_addr32[1] = 0;
> +    a2.s6_addr32[2] = htonl(0xffff);
> +    a2.s6_addr32[3] = (uint32_t)a1.s_addr;
> +}

If you use ipv6_addr_set() everywhere you don't need this.

> static inline int ipv6_addr_v4mapped(const struct in6_addr *a)
> {
>     return ((a->s6_addr32[0] | a->s6_addr32[1]) == 0 &&
> -         a->s6_addr32[2] == htonl(0x0000ffff));
> +        a->s6_addr32[2] == htonl(0x0000ffff));
> }

Guessing you changed a tab to a space, unnecessary.

> -static struct ip_map *ip_map_lookup(char *class, struct in_addr addr);
> +static struct ip_map *ip_map_lookup(char *class, struct in6_addr addr);

I still think you should pass a pointer here.

> -int auth_unix_add_addr(struct in_addr addr, struct auth_domain *dom)
> +int auth_unix_add_addr(struct in6_addr addr, struct auth_domain *dom)

And here.

> -struct auth_domain *auth_unix_lookup(struct in_addr addr)
> +struct auth_domain *auth_unix_lookup(struct in6_addr addr)

And here.

-Brian