lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 07 May 2014 09:56:56 -0400 From: Vlad Yasevich <vyasevic@...hat.com> To: Veaceslav Falico <vfalico@...hat.com> CC: netdev@...r.kernel.org, Jay Vosburgh <fubar@...ibm.com>, Andy Gospodarek <andy@...yhouse.net>, Ding Tianhong <dingtianhong@...wei.com>, Patric McHardy <kaber@...sh.net> Subject: Re: [PATCH net] bonding: Fix stacked device detection in arp monitoring On 05/07/2014 09:32 AM, Veaceslav Falico wrote: > On Tue, May 06, 2014 at 01:41:15PM -0400, Vlad Yasevich wrote: >> Prior to commit fbd929f2dce460456807a51e18d623db3db9f077 >> bonding: support QinQ for bond arp interval >> >> the arp monitoring code allowed for proper detection of devices >> stacked on top of vlans. Since the above commit, the >> code can still detect a device stacked on top of single >> vlan, but not a device stacked on top of Q-in-Q configuration. >> The search will only set the inner vlan tag if the route >> device is the vlan device. However, this is not always the >> case, as it is possible to extend the stacked configuration. >> >> With this patch it is possible to provision devices on >> top Q-in-Q vlan configuration that should be used as >> a source of ARP monitoring information. >> >> For example: >> ip link add link bond0 vlan10 type vlan proto 802.1q id 10 >> ip link add link vlan10 vlan100 type vlan proto 802.1q id 100 >> ip link add link vlan100 type macvlan >> >> Note: This patch limits the number of stacked VLANs to 2, >> just like before. The original, however had another issue >> in that if we had more then 2 levels of VLANs, we would end >> up generating incorrectly tagged traffic. This is no longer >> possible. >> >> Fixes: fbd929f2dce460456807a51e18d623db3db9f077 (bonding: support QinQ >> for bond arp interval) >> CC: Jay Vosburgh <fubar@...ibm.com> >> CC: Veaceslav Falico <vfalico@...hat.com> >> CC: Andy Gospodarek <andy@...yhouse.net> >> CC: Ding Tianhong <dingtianhong@...wei.com> >> CC: Patric McHardy <kaber@...sh.net> >> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com> >> --- >> drivers/net/bonding/bond_main.c | 90 >> +++++++++++++++++------------------------ >> 1 file changed, 37 insertions(+), 53 deletions(-) >> >> diff --git a/drivers/net/bonding/bond_main.c >> b/drivers/net/bonding/bond_main.c >> index 9d08e00..4822728 100644 >> --- a/drivers/net/bonding/bond_main.c >> +++ b/drivers/net/bonding/bond_main.c >> @@ -2164,22 +2164,46 @@ static void bond_arp_send(struct net_device >> *slave_dev, int arp_op, >> arp_xmit(skb); >> } >> >> +static bool bond_collect_vlans(struct net_device *start, struct >> net_device *end, >> + struct bond_vlan_tag *tag, int idx) >> +{ >> + struct net_device *upper; >> + struct list_head *iter; >> + >> + /* We do not support more then 2 levels of VLAN nesting */ >> + if (idx >= 2) >> + return false; >> + >> + netdev_for_each_all_upper_dev_rcu(start, upper, iter) { >> + if (is_vlan_dev(upper)) { >> + tag[idx].vlan_proto = vlan_dev_vlan_proto(upper); >> + tag[idx].vlan_id = vlan_dev_vlan_id(upper); >> + idx++; > > Won't idx here skyrocket if we have more than 1 vlan on top of bonding? Right. See v2 of the patch. :) Thanks -vlad > > Otherwise I like this approach, from the first view. > >> + } >> + if (upper == end) >> + return true; >> + >> + /* Look at upper devices list only if current device >> + * is a vlan >> + */ >> + if (is_vlan_dev(upper) && bond_collect_vlans(upper, end, tag, >> idx)) >> + return true; >> + } >> + return false; >> +} >> + >> >> static void bond_arp_send_all(struct bonding *bond, struct slave *slave) >> { >> - struct net_device *upper, *vlan_upper; >> - struct list_head *iter, *vlan_iter; >> struct rtable *rt; >> - struct bond_vlan_tag inner, outer; >> + struct bond_vlan_tag tags[2]; >> __be32 *targets = bond->params.arp_targets, addr; >> int i; >> + bool ret; >> >> for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) { >> pr_debug("basa: target %pI4\n", &targets[i]); >> - inner.vlan_proto = 0; >> - inner.vlan_id = 0; >> - outer.vlan_proto = 0; >> - outer.vlan_id = 0; >> + memset(tags, 0, sizeof(tags)); >> >> /* Find out through which dev should the packet go */ >> rt = ip_route_output(dev_net(bond->dev), targets[i], 0, >> @@ -2192,7 +2216,7 @@ static void bond_arp_send_all(struct bonding >> *bond, struct slave *slave) >> net_warn_ratelimited("%s: no route to arp_ip_target >> %pI4 and arp_validate is set\n", >> bond->dev->name, >> &targets[i]); >> - bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], 0, >> &inner, &outer); >> + bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], 0, >> &tags[1], &tags[0]); >> continue; >> } >> >> @@ -2201,52 +2225,12 @@ static void bond_arp_send_all(struct bonding >> *bond, struct slave *slave) >> goto found; >> >> rcu_read_lock(); >> - /* first we search only for vlan devices. for every vlan >> - * found we verify its upper dev list, searching for the >> - * rt->dst.dev. If found we save the tag of the vlan and >> - * proceed to send the packet. >> - */ >> - netdev_for_each_all_upper_dev_rcu(bond->dev, vlan_upper, >> - vlan_iter) { >> - if (!is_vlan_dev(vlan_upper)) >> - continue; >> - >> - if (vlan_upper == rt->dst.dev) { >> - outer.vlan_proto = vlan_dev_vlan_proto(vlan_upper); >> - outer.vlan_id = vlan_dev_vlan_id(vlan_upper); >> - rcu_read_unlock(); >> - goto found; >> - } >> - netdev_for_each_all_upper_dev_rcu(vlan_upper, upper, >> - iter) { >> - if (upper == rt->dst.dev) { >> - /* If the upper dev is a vlan dev too, >> - * set the vlan tag to inner tag. >> - */ >> - if (is_vlan_dev(upper)) { >> - inner.vlan_proto = vlan_dev_vlan_proto(upper); >> - inner.vlan_id = vlan_dev_vlan_id(upper); >> - } >> - outer.vlan_proto = vlan_dev_vlan_proto(vlan_upper); >> - outer.vlan_id = vlan_dev_vlan_id(vlan_upper); >> - rcu_read_unlock(); >> - goto found; >> - } >> - } >> - } >> - >> - /* if the device we're looking for is not on top of any of >> - * our upper vlans, then just search for any dev that >> - * matches, and in case it's a vlan - save the id >> - */ >> - netdev_for_each_all_upper_dev_rcu(bond->dev, upper, iter) { >> - if (upper == rt->dst.dev) { >> - rcu_read_unlock(); >> - goto found; >> - } >> - } >> + ret = bond_collect_vlans(bond->dev, rt->dst.dev, tags, 0); >> rcu_read_unlock(); >> >> + if (ret) >> + goto found; >> + >> /* Not our device - skip */ >> pr_debug("%s: no path to arp_ip_target %pI4 via rt.dev %s\n", >> bond->dev->name, &targets[i], >> @@ -2259,7 +2243,7 @@ found: >> addr = bond_confirm_addr(rt->dst.dev, targets[i], 0); >> ip_rt_put(rt); >> bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i], >> - addr, &inner, &outer); >> + addr, &tags[1], &tags[0]); >> } >> } >> >> -- >> 1.9.0 >> -- 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