[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1427918823.3025.22.camel@redhat.com>
Date: Wed, 01 Apr 2015 15:07:03 -0500
From: Dan Williams <dcbw@...hat.com>
To: Mahesh Bandewar <maheshb@...gle.com>
Cc: linux-netdev <netdev@...r.kernel.org>, jbenc@...hat.com
Subject: Re: [PATCH] ipvlan: fix up broadcast MAC filtering for ARP and DHCP
On Mon, 2015-03-30 at 23:22 -0500, Dan Williams wrote:
> The broadcast MAC is supposed to be allowed whenever the device
> has an IPv4 address, otherwise ARP requests get dropped on the
> floor. If ndo_set_rx_mode (and thus
> ipvlan_set_multicast_mac_filter()) got called after the address
> was added, it would blow away the broadcast MAC address in
> mac_filters that was added at IPv4 address addition.
>
> Next, ipvlan currently fails DHCP addressing for two reasons:
>
> 1) DHCP offers are typically unicast back to the device's MAC
> address, and at the IP layer have a destination IP address
> matching the new lease address. In ipvlan unicast packets
> are checked against existing addresses assigned to the ipvlan
> interface, so clearly this fails hard because the ipvlan
> interface has no IP addresses yet. Workaround: request
> that the server broadcast replies (-B for dhclient) which
> don't get checked against the address list.
>
> 2) Even when that's done, mac_filters only allows the
> broadcast MAC address when the interface has >= 1 IPv4
> addresses, so double-fail, and the incoming DHCP offer
> gets dropped on the floor again.
>
> To fix these issues, Watch for outgoing DHCP requests and
> enable the broadcast MAC address indefinitely when one is seen.
Any thoughts on this approach Mahesh?
Dan
> Signed-off-by: Dan Williams <dcbw@...hat.com>
> ---
> NOTE: this patch supercedes my previous two ipvlan patches:
> [PATCH 1/2] ipvlan: don't loose broadcast MAC when setting MAC filters
> [PATCH 2/2] ipvlan: always allow the broadcast MAC address
>
> drivers/net/ipvlan/ipvlan.h | 2 ++
> drivers/net/ipvlan/ipvlan_core.c | 36 ++++++++++++++++++++++++++++++++++--
> drivers/net/ipvlan/ipvlan_main.c | 12 +++++++++---
> 3 files changed, 45 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan.h b/drivers/net/ipvlan/ipvlan.h
> index 924ea98..7e0b8ff 100644
> --- a/drivers/net/ipvlan/ipvlan.h
> +++ b/drivers/net/ipvlan/ipvlan.h
> @@ -67,6 +67,7 @@ struct ipvl_dev {
> struct list_head addrs;
> int ipv4cnt;
> int ipv6cnt;
> + bool dhcp4_seen;
> struct ipvl_pcpu_stats __percpu *pcpu_stats;
> DECLARE_BITMAP(mac_filters, IPVLAN_MAC_FILTER_SIZE);
> netdev_features_t sfeatures;
> @@ -118,4 +119,5 @@ bool ipvlan_addr_busy(struct ipvl_dev *ipvlan, void *iaddr, bool is_v6);
> struct ipvl_addr *ipvlan_ht_addr_lookup(const struct ipvl_port *port,
> const void *iaddr, bool is_v6);
> void ipvlan_ht_addr_del(struct ipvl_addr *addr, bool sync);
> +void ipvlan_set_broadcast_mac_filter(struct ipvl_dev *ipvlan, bool set);
> #endif /* __IPVLAN_H */
> diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c
> index 2a17500..6dd992b 100644
> --- a/drivers/net/ipvlan/ipvlan_core.c
> +++ b/drivers/net/ipvlan/ipvlan_core.c
> @@ -126,6 +126,12 @@ static void *ipvlan_get_L3_hdr(struct sk_buff *skb, int *type)
> lyr3h = arph;
> break;
> }
> + case htons(ETH_P_ALL): {
> + /* Raw sockets */
> + if (eth_hdr(skb)->h_proto != htons(ETH_P_IP))
> + break;
> + /* Fall through */
> + }
> case htons(ETH_P_IP): {
> u32 pktlen;
> struct iphdr *ip4h;
> @@ -454,16 +460,42 @@ out:
> return ipvlan_process_outbound(skb, ipvlan);
> }
>
> +#define DHCP_PACKET_MIN_LEN 282
> +
> +static bool is_dhcp(struct sk_buff *skb)
> +{
> + struct iphdr *ip4h = ip_hdr(skb);
> + struct udphdr *udph;
> +
> + if (skb->len < DHCP_PACKET_MIN_LEN || ip4h->protocol != IPPROTO_UDP
> + || ip_is_fragment(ip4h))
> + return false;
> +
> + /* In the case of RAW sockets the transport header is not set by
> + * the IP stack so we must set it ourselves.
> + */
> + if (skb->transport_header == skb->network_header)
> + skb_set_transport_header(skb, ETH_HLEN + sizeof(struct iphdr));
> + udph = udp_hdr(skb);
> + return udph && udph->dest == htons(67) && udph->source == htons(68);
> +}
> +
> static int ipvlan_xmit_mode_l2(struct sk_buff *skb, struct net_device *dev)
> {
> - const struct ipvl_dev *ipvlan = netdev_priv(dev);
> + struct ipvl_dev *ipvlan = netdev_priv(dev);
> struct ethhdr *eth = eth_hdr(skb);
> struct ipvl_addr *addr;
> void *lyr3h;
> int addr_type;
>
> + lyr3h = ipvlan_get_L3_hdr(skb, &addr_type);
> + if (!ipvlan->dhcp4_seen && lyr3h && addr_type == IPVL_IPV4
> + && is_dhcp(skb)) {
> + ipvlan->dhcp4_seen = true;
> + ipvlan_set_broadcast_mac_filter(ipvlan, true);
> + }
> +
> if (ether_addr_equal(eth->h_dest, eth->h_source)) {
> - lyr3h = ipvlan_get_L3_hdr(skb, &addr_type);
> if (lyr3h) {
> addr = ipvlan_addr_lookup(ipvlan->port, lyr3h, addr_type, true);
> if (addr)
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 4f4099d..a497fb9 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -167,6 +167,8 @@ static int ipvlan_stop(struct net_device *dev)
>
> dev_uc_del(phy_dev, phy_dev->dev_addr);
>
> + ipvlan->dhcp4_seen = false;
> +
> if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
> list_for_each_entry(addr, &ipvlan->addrs, anode)
> ipvlan_ht_addr_del(addr, !dev->dismantle);
> @@ -214,7 +216,7 @@ static void ipvlan_change_rx_flags(struct net_device *dev, int change)
> dev_set_allmulti(phy_dev, dev->flags & IFF_ALLMULTI? 1 : -1);
> }
>
> -static void ipvlan_set_broadcast_mac_filter(struct ipvl_dev *ipvlan, bool set)
> +void ipvlan_set_broadcast_mac_filter(struct ipvl_dev *ipvlan, bool set)
> {
> struct net_device *dev = ipvlan->dev;
> unsigned int hashbit = ipvlan_mac_hash(dev->broadcast);
> @@ -239,6 +241,9 @@ static void ipvlan_set_multicast_mac_filter(struct net_device *dev)
> netdev_for_each_mc_addr(ha, dev)
> __set_bit(ipvlan_mac_hash(ha->addr), mc_filters);
>
> + if (ipvlan->ipv4cnt || ipvlan->dhcp4_seen)
> + __set_bit(ipvlan_mac_hash(dev->broadcast), mc_filters);
> +
> bitmap_copy(ipvlan->mac_filters, mc_filters,
> IPVLAN_MAC_FILTER_SIZE);
> }
> @@ -467,6 +472,7 @@ static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> INIT_LIST_HEAD(&ipvlan->addrs);
> ipvlan->ipv4cnt = 0;
> ipvlan->ipv6cnt = 0;
> + ipvlan->dhcp4_seen = false;
>
> /* TODO Probably put random address here to be presented to the
> * world but keep using the physical-dev address for the outgoing
> @@ -708,8 +714,8 @@ static void ipvlan_del_addr4(struct ipvl_dev *ipvlan, struct in_addr *ip4_addr)
> list_del_rcu(&addr->anode);
> ipvlan->ipv4cnt--;
> WARN_ON(ipvlan->ipv4cnt < 0);
> - if (!ipvlan->ipv4cnt)
> - ipvlan_set_broadcast_mac_filter(ipvlan, false);
> + if (!ipvlan->ipv4cnt && !ipvlan->dhcp4_seen)
> + ipvlan_set_broadcast_mac_filter(ipvlan, false);
> kfree_rcu(addr, rcu);
>
> return;
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists