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

From: Hideaki Yoshifuji
Date: Mon Nov 23 2020 - 08:27:58 EST

Next message: Catalin Marinas: "Re: [PATCH 1/6] arm64: pgtable: Fix pte_accessible()"
Previous message: Greg Kroah-Hartman: "[PATCH 4.19 36/91] Revert "arm: sun8i: orangepi-pc-plus: Set EMAC activity LEDs to active high""
In reply to: Dmytro Shytyi: "[PATCH net-next V7] net: Variable SLAAC: SLAAC with prefixes of arbitrary length in PIO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi,

2020年11月20日(金) 18:28 Dmytro Shytyi <dmytro@xxxxxxxxxx>:
>
> Variable SLAAC: SLAAC with prefixes of arbitrary length in PIO (randomly
> generated hostID or stable privacy + privacy extensions).
> The main problem is that SLAAC RA or PD allocates a /64 by the Wireless
> carrier 4G, 5G to a mobile hotspot, however segmentation of the /64 via
> SLAAC is required so that downstream interfaces can be further subnetted.
> Example: uCPE device (4G + WI-FI enabled) receives /64 via Wireless, and
> assigns /72 to VNF-Firewall, /72 to WIFI, /72 to VNF-Router, /72 to
> Load-Balancer and /72 to wired connected devices.
> IETF document that defines problem statement:
> draft-mishra-v6ops-variable-slaac-problem-stmt
> IETF document that specifies variable slaac:
> draft-mishra-6man-variable-slaac
>
> Signed-off-by: Dmytro Shytyi <dmytro@xxxxxxxxxx>
> ---
> diff -rupN net-next-5.10.0-rc2/net/ipv6/addrconf.c net-next-patch-5.10.0-rc2/net/ipv6/addrconf.c
> --- net-next-5.10.0-rc2/net/ipv6/addrconf.c 2020-11-10 08:46:01.075193379 +0100
> +++ net-next-patch-5.10.0-rc2/net/ipv6/addrconf.c 2020-11-19 21:26:39.770872898 +0100
> @@ -142,7 +142,6 @@ static int ipv6_count_addresses(const st
> static int ipv6_generate_stable_address(struct in6_addr *addr,
> u8 dad_count,
> const struct inet6_dev *idev);
> -

Do not remove this line.
> #define IN6_ADDR_HSIZE_SHIFT 8
> #define IN6_ADDR_HSIZE (1 << IN6_ADDR_HSIZE_SHIFT)
> /*
> @@ -1315,6 +1314,7 @@ static int ipv6_create_tempaddr(struct i
> struct ifa6_config cfg;
> long max_desync_factor;
> struct in6_addr addr;
> + struct in6_addr temp;
> int ret = 0;
>
> write_lock_bh(&idev->lock);
> @@ -1340,9 +1340,16 @@ retry:
> goto out;
> }
> in6_ifa_hold(ifp);
> - memcpy(addr.s6_addr, ifp->addr.s6_addr, 8);
> - ipv6_gen_rnd_iid(&addr);
>
> + if (ifp->prefix_len == 64) {
> + memcpy(addr.s6_addr, ifp->addr.s6_addr, 8);
> + ipv6_gen_rnd_iid(&addr);
> + } else if (ifp->prefix_len > 0 && ifp->prefix_len <= 128) {
> + memcpy(addr.s6_addr32, ifp->addr.s6_addr, 16);
> + get_random_bytes(temp.s6_addr32, 16);
> + ipv6_addr_prefix_copy(&temp, &addr, ifp->prefix_len);
> + memcpy(addr.s6_addr, temp.s6_addr, 16);
> + }

I do not understand why you are copying many times.
ipv6_addr_copy(), get_random_bytes(), and then ipv6_addr_prefix_copy
is enough, no?

> age = (now - ifp->tstamp) / HZ;
>
> regen_advance = idev->cnf.regen_max_retry *
> @@ -2569,6 +2576,41 @@ static bool is_addr_mode_generate_stable
> idev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_RANDOM;
> }
>
> +static struct inet6_ifaddr *ipv6_cmp_rcvd_prsnt_prfxs(struct inet6_ifaddr *ifp,
> + struct inet6_dev *in6_dev,
> + struct net *net,
> + const struct prefix_info *pinfo)
> +{
> + struct inet6_ifaddr *result_base = NULL;
> + struct inet6_ifaddr *result = 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;
> + }

I guess we can compare them with ipv6_prefix_equal()
directly because the code assumes "pinfo->prefix_len" and ifp->prefix_len are
equal.

> + }
> + rcu_read_unlock();
> + if (result_base != result)
> + ifp = result;
> + else
> + ifp = NULL;
> +
> + return ifp;
> +}
> +
> int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
> const struct prefix_info *pinfo,
> struct inet6_dev *in6_dev,
> @@ -2576,9 +2618,16 @@ 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 plen = pinfo->prefix_len;
> int create = 0;
>
> + if (plen > 0 && plen <= 128 && plen != 64 &&
> + in6_dev->cnf.addr_gen_mode != IN6_ADDR_GEN_MODE_STABLE_PRIVACY)
> + ifp = ipv6_cmp_rcvd_prsnt_prfxs(ifp, in6_dev, net, pinfo);
> + else
> + ifp = ipv6_get_ifaddr(net, addr, dev, 1);
> +
> if (!ifp && valid_lft) {
> int max_addresses = in6_dev->cnf.max_addresses;
> struct ifa6_config cfg = {

I am wondering if we should enable this feature by default at this moment
because the spec is personal internet draft and some test suites might
consider this feature violates standards.

> @@ -2657,6 +2706,91 @@ int addrconf_prefix_rcv_add_addr(struct
> }
> EXPORT_SYMBOL_GPL(addrconf_prefix_rcv_add_addr);
>
> +static bool ipv6_reserved_interfaceid(struct in6_addr address)
> +{
> + if ((address.s6_addr32[2] | address.s6_addr32[3]) == 0)
> + return true;
> +
> + if (address.s6_addr32[2] == htonl(0x02005eff) &&
> + ((address.s6_addr32[3] & htonl(0xfe000000)) == htonl(0xfe000000)))
> + return true;
> +
> + if (address.s6_addr32[2] == htonl(0xfdffffff) &&
> + ((address.s6_addr32[3] & htonl(0xffffff80)) == htonl(0xffffff80)))
> + return true;
> +
> + return false;
> +}
> +
> +static int ipv6_gen_addr_var_plen(struct in6_addr *address,
> + u8 dad_count,
> + const struct inet6_dev *idev,
> + unsigned int rcvd_prfx_len,
> + bool stable_privacy_mode)
> +{
> + static union {
> + char __data[SHA1_BLOCK_SIZE];
> + struct {
> + struct in6_addr secret;
> + __be32 prefix[2];
> + unsigned char hwaddr[MAX_ADDR_LEN];
> + u8 dad_count;
> + } __packed;
> + } data;
> + static __u32 workspace[SHA1_WORKSPACE_WORDS];
> + static __u32 digest[SHA1_DIGEST_WORDS];
> + struct net *net = dev_net(idev->dev);
> + static DEFINE_SPINLOCK(lock);
> + struct in6_addr secret;
> + struct in6_addr temp;
> +
> + BUILD_BUG_ON(sizeof(data.__data) != sizeof(data));
> +
> + if (stable_privacy_mode) {
> + if (idev->cnf.stable_secret.initialized)
> + secret = idev->cnf.stable_secret.secret;
> + else if (net->ipv6.devconf_dflt->stable_secret.initialized)
> + secret = net->ipv6.devconf_dflt->stable_secret.secret;
> + else
> + return -1;
> + }
> +
> +retry:
> + spin_lock_bh(&lock);
> + if (stable_privacy_mode) {
> + sha1_init(digest);
> + memset(&data, 0, sizeof(data));
> + memset(workspace, 0, sizeof(workspace));
> + memcpy(data.hwaddr, idev->dev->perm_addr, idev->dev->addr_len);
> + data.prefix[0] = address->s6_addr32[0];
> + data.prefix[1] = address->s6_addr32[1];
> + data.secret = secret;
> + data.dad_count = dad_count;
> +
> + sha1_transform(digest, data.__data, workspace);
> +
> + temp = *address;
> + temp.s6_addr32[0] = (__force __be32)digest[0];
> + temp.s6_addr32[1] = (__force __be32)digest[1];
> + temp.s6_addr32[2] = (__force __be32)digest[2];
> + temp.s6_addr32[3] = (__force __be32)digest[3];

I do not understand why you copy *address and then overwrite
by digest?

> + } else {
> + temp = *address;
> + get_random_bytes(temp.s6_addr32, 16);
> + }
> + spin_unlock_bh(&lock);
> +
> + if (ipv6_reserved_interfaceid(temp)) {
> + dad_count++;
> + if (dad_count > dev_net(idev->dev)->ipv6.sysctl.idgen_retries)
> + return -1;
> + goto retry;
> + }
> + ipv6_addr_prefix_copy(&temp, address, rcvd_prfx_len);
> + *address = temp;
> + return 0;
> +}
> +
> void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
> {
> struct prefix_info *pinfo;
> @@ -2781,9 +2915,33 @@ void addrconf_prefix_rcv(struct net_devi
> dev_addr_generated = true;
> }
> goto ok;
> + } else if (pinfo->prefix_len != 64 &&
> + pinfo->prefix_len > 0 && pinfo->prefix_len <= 128) {
> + /* SLAAC with prefixes of arbitrary length (Variable SLAAC).
> + * draft-mishra-6man-variable-slaac
> + * draft-mishra-v6ops-variable-slaac-problem-stmt
> + */
> + memcpy(&addr, &pinfo->prefix, 16);
> + if (in6_dev->cnf.addr_gen_mode == IN6_ADDR_GEN_MODE_STABLE_PRIVACY) {
> + if (!ipv6_gen_addr_var_plen(&addr,
> + 0,
> + in6_dev,
> + pinfo->prefix_len,
> + true)) {
> + addr_flags |= IFA_F_STABLE_PRIVACY;
> + goto ok;
> + }
> + } else if (!ipv6_gen_addr_var_plen(&addr,
> + 0,
> + in6_dev,
> + pinfo->prefix_len,
> + false)) {
> + goto ok;
> + }
> + } else {
> + net_dbg_ratelimited("IPv6: Prefix with unexpected length %d\n",
> + pinfo->prefix_len);
> }
> - net_dbg_ratelimited("IPv6 addrconf: prefix with wrong length %d\n",
> - pinfo->prefix_len);
> goto put;
>
> ok:
> @@ -3186,22 +3344,6 @@ void addrconf_add_linklocal(struct inet6
> }
> EXPORT_SYMBOL_GPL(addrconf_add_linklocal);
>
> -static bool ipv6_reserved_interfaceid(struct in6_addr address)
> -{
> - if ((address.s6_addr32[2] | address.s6_addr32[3]) == 0)
> - return true;
> -
> - if (address.s6_addr32[2] == htonl(0x02005eff) &&
> - ((address.s6_addr32[3] & htonl(0xfe000000)) == htonl(0xfe000000)))
> - return true;
> -
> - if (address.s6_addr32[2] == htonl(0xfdffffff) &&
> - ((address.s6_addr32[3] & htonl(0xffffff80)) == htonl(0xffffff80)))
> - return true;
> -
> - return false;
> -}
> -
> static int ipv6_generate_stable_address(struct in6_addr *address,
> u8 dad_count,
> const struct inet6_dev *idev)

Next message: Catalin Marinas: "Re: [PATCH 1/6] arm64: pgtable: Fix pte_accessible()"
Previous message: Greg Kroah-Hartman: "[PATCH 4.19 36/91] Revert "arm: sun8i: orangepi-pc-plus: Set EMAC activity LEDs to active high""
In reply to: Dmytro Shytyi: "[PATCH net-next V7] net: Variable SLAAC: SLAAC with prefixes of arbitrary length in PIO"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]