| 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, 7 May 2014 15:32:39 +0200
From: Veaceslav Falico <vfalico@...hat.com>
To: Vlad Yasevich <vyasevic@...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 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?
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