[PATCHv3,net-next] VXLAN: fix nonfunctional neigh_reduce

The VXLAN neigh_reduce() code is completely non-functional since
check-in. Specific errors:

1) The original code drops all packets with a multicast destination address,
	even though neighbor solicitations are sent to the solicited-node
	address, a multicast address. The code after this check was never run.
2) The neighbor table lookup used the IPv6 header destination, which is the
	solicited node address, rather than the target address from the
	neighbor solicitation. So neighbor lookups would always fail if it
	got this far. Also for L3MISSes.
3) The code calls ndisc_send_na(), which does a send on the tunnel device.
	The context for neigh_reduce() is the transmit path, vxlan_xmit(),
	where the host or a bridge-attached neighbor is trying to transmit
	a neighbor solicitation. To respond to it, the tunnel endpoint needs
	to do a *receive* of the appropriate neighbor advertisement. Doing a
	send, would only try to send the advertisement, encapsulated, to the
	remote destinations in the fdb -- hosts that definitely did not do the
	corresponding solicitation.
4) The code uses the tunnel endpoint IPv6 forwarding flag to determine the
	isrouter flag in the advertisement. This has nothing to do with whether
	or not the target is a router, and generally won't be set since the
	tunnel endpoint is bridging, not routing, traffic.

	The patch below creates a proxy neighbor advertisement to respond to
neighbor solicitions as intended, providing proper IPv6 support for neighbor
reduction.

Changes since v2:

- code cleanup suggested by Stephen Hemminger and Daniel Baluta

Changes since v1:

- reworked code to be structurally similar to arp_reduce() per Dave Miller

Signed-Off-By: David L Stevens <dlstevens@us.ibm.com>


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Mar 18, 2014 at 9:20 AM, David L Stevens <dlstevens@us.ibm.com> wrote:
>
>         The VXLAN neigh_reduce() code is completely non-functional since
> check-in. Specific errors:
>
> 1) The original code drops all packets with a multicast destination address,
>         even though neighbor solicitations are sent to the solicited-node
>         address, a multicast address. The code after this check was never run.
> 2) The neighbor table lookup used the IPv6 header destination, which is the
>         solicited node address, rather than the target address from the
>         neighbor solicitation. So neighbor lookups would always fail if it
>         got this far. Also for L3MISSes.
> 3) The code calls ndisc_send_na(), which does a send on the tunnel device.
>         The context for neigh_reduce() is the transmit path, vxlan_xmit(),
>         where the host or a bridge-attached neighbor is trying to transmit
>         a neighbor solicitation. To respond to it, the tunnel endpoint needs
>         to do a *receive* of the appropriate neighbor advertisement. Doing a
>         send, would only try to send the advertisement, encapsulated, to the
>         remote destinations in the fdb -- hosts that definitely did not do the
>         corresponding solicitation.
> 4) The code uses the tunnel endpoint IPv6 forwarding flag to determine the
>         isrouter flag in the advertisement. This has nothing to do with whether
>         or not the target is a router, and generally won't be set since the
>         tunnel endpoint is bridging, not routing, traffic.
>
>         The patch below creates a proxy neighbor advertisement to respond to
> neighbor solicitions as intended, providing proper IPv6 support for neighbor
> reduction.
>
> Changes since v2:
>
> - code cleanup suggested by Stephen Hemminger and Daniel Baluta
>
> Changes since v1:
>
> - reworked code to be structurally similar to arp_reduce() per Dave Miller
>
> Signed-Off-By: David L Stevens <dlstevens@us.ibm.com>

I thought ipv6_stub->ndisc_send_na can be removed, but it is being
used by other driver. So your patch looks good to me:

Reviewed-by: Cong Wang <cwang@twopensource.com>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 03/18/2014 12:20 PM, David L Stevens wrote:
> 
> 	The VXLAN neigh_reduce() code is completely non-functional since
> check-in. Specific errors:
<snip>
> 
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index eb59b14..f6ddde9 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1336,14 +1336,102 @@ out:
>  }
>  
>  #if IS_ENABLED(CONFIG_IPV6)
> +
> +static struct sk_buff *vxlan_na_create(struct sk_buff *request,
> +	struct neighbour *n, bool isrouter)
> +{
> +	struct net_device *dev = request->dev;
> +	struct sk_buff *reply = NULL;

Don't need this initialization as it's assigned below before using.

> +	struct nd_msg *ns, *na;
> +	struct ipv6hdr *pip6;
> +	u8 *daddr;
> +	int olen = 8; /* opt hdr + ETH_ALEN for target */
> +	int i, len;
> +
> +	if (dev == NULL)
> +		return NULL;
> +
> +	ns = (struct nd_msg *)skb_transport_header(request);

Nit: don't use this until below, can move initialization.

> +	len = LL_RESERVED_SPACE(dev) + sizeof(struct ipv6hdr) +
> +		sizeof(*na) + olen + dev->needed_tailroom;
> +	reply = alloc_skb(len, GFP_ATOMIC);
> +	if (reply == NULL)
> +		return NULL;
> +
> +	reply->protocol = htons(ETH_P_IPV6);
> +	reply->dev = dev;
> +	skb_reserve(reply, LL_RESERVED_SPACE(request->dev));
> +	skb_push(reply, sizeof(struct ethhdr));
> +	skb_set_mac_header(reply, 0);
> +
> +	daddr = eth_hdr(request)->h_source;
> +	olen = request->len - skb_transport_offset(request) - sizeof(*ns);
> +	for (i = 0; i < olen-1; i += (ns->opt[i+1]<<3)) {
> +		if (ns->opt[i] == ND_OPT_SOURCE_LL_ADDR) {
> +			daddr = ns->opt + i + sizeof(struct nd_opt_hdr);
> +			break;
> +		}
> +	}
> +
> +	/* Ethernet header */
> +	memcpy(eth_hdr(reply)->h_dest, daddr, ETH_ALEN);
> +	memcpy(eth_hdr(reply)->h_source, n->ha, ETH_ALEN);

Can use ether_addr_copy() here.

-Brian
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html