[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150715044521.GA7603@vergenet.net>
Date: Wed, 15 Jul 2015 13:45:23 +0900
From: Simon Horman <simon.horman@...ronome.com>
To: Scott Feldman <sfeldma@...il.com>
Cc: Jiri Pirko <jiri@...nulli.us>, Netdev <netdev@...r.kernel.org>,
john fastabend <john.fastabend@...il.com>
Subject: Re: [PATCH/RFC net-next] rocker: forward packets to CPU when a port
in promiscuous mode
Hi Scott,
On Mon, Jul 13, 2015 at 11:37:59PM -0700, Scott Feldman wrote:
> On Wed, Jul 8, 2015 at 9:25 PM, Simon Horman <simon.horman@...ronome.com> wrote:
> > This change allows the CPU to see all packets seen by a port when the
> > netdev associated with the port is in promiscuous mode.
> >
> > This change was previously posted as part of a larger patch and in turn
> > patchset which also aimed to allow rocker interfaces to receive packets
> > when not bridged. That problem has subsequently been addressed in a
> > different way by Scott Feldman.
> >
> > When this change was previously posted Scott indicated that he had some
> > reservations about sending all packets from a switch to the CPU. The
> > purpose of posting this patch is to start discussion of weather this
> > approach is appropriate and if not how else we might move forwards.
> >
> > In my opinion if host doesn't want all packets its shouldn't put a port
> > in promiscuous mode. But perhaps that is an overly naïve view to take.
> >
> > My main motivation for this change at this time is to allow rocker to
> > work with Open vSwitch and it appears that this change is sufficient to
> > reach that goal. Another approach might be to teach
> > rocker_port_master_changed() about Open vSwitch.
> >
> > In the longer term I believe Open vSwitch should be able to program
> > flows into rocker 'hardware' and thus not all packets would reach the CPU.
>
> Hi Simon,
>
> I like your alternate approach to teach rocker about Open vSwitch
> using rocker_port_master_change() and only when port is captured by
> OVS would we install the "promisc" filter to pass all traffic up.
> (Maybe call it ROCKER_CTRL_DFLT_OVS rule?).
>
> Putting a non-bridged, non-ovs port into promisc is kind of weird for
> a switch port. I think of the port in L3 mode by default, where the
> port is locked down for all but some selective mcasts, and only opened
> up by installing explicit routes. (Unlike a bridged port where we
> flood everything L2 we don't understand).
>
> So maybe first pass is to pass up everything when port is captured by
> OVS, and then later refine what's passed up per ovs flows on that
> port.
That sounds reasonable to me. Its pretty clear to me from the responses
from John and yourself that my approach to the promiscuous flag isn't
as clean-cut as I had hoped. And it seems that we have a nice way to
move forwards on supporting Open vSwitch.
How about this?
-- >8 --
From: Simon Horman <simon.horman@...ronome.com>
Subject: rocker: forward packets to CPU when port is joined to openvswitch
Teach rocker to forward packets to CPU when a port is joined to Open vSwitch.
There is scope to later refine what is passed up as per Open vSwitch flows
on a port.
This does not change the behaviour of rocker ports that are
not joined to Open vSwitch.
Signed-off-by: Simon Horman <simon.horman@...ronome.com>
---
drivers/net/ethernet/rocker/rocker.c | 55 ++++++++++++++++++++++++++++++++----
1 file changed, 49 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
index c0051673c9fa..8c53d6839260 100644
--- a/drivers/net/ethernet/rocker/rocker.c
+++ b/drivers/net/ethernet/rocker/rocker.c
@@ -202,6 +202,7 @@ enum {
ROCKER_CTRL_IPV4_MCAST,
ROCKER_CTRL_IPV6_MCAST,
ROCKER_CTRL_DFLT_BRIDGING,
+ ROCKER_CTRL_DFLT_OVS,
ROCKER_CTRL_MAX,
};
@@ -321,9 +322,21 @@ static u16 rocker_port_vlan_to_vid(const struct rocker_port *rocker_port,
return ntohs(vlan_id);
}
+static bool rocker_port_is_bridged__(const struct rocker_port *rocker_port,
+ const char *kind)
+{
+ return rocker_port->bridge_dev &&
+ !strcmp(rocker_port->bridge_dev->rtnl_link_ops->kind, kind);
+}
+
static bool rocker_port_is_bridged(const struct rocker_port *rocker_port)
{
- return !!rocker_port->bridge_dev;
+ return rocker_port_is_bridged__(rocker_port, "bridge");
+}
+
+static bool rocker_port_is_ovs(const struct rocker_port *rocker_port)
+{
+ return rocker_port_is_bridged__(rocker_port, "openvswitch");
}
#define ROCKER_OP_FLAG_REMOVE BIT(0)
@@ -3275,6 +3288,12 @@ static struct rocker_ctrl {
.bridge = true,
.copy_to_cpu = true,
},
+ [ROCKER_CTRL_DFLT_OVS] = {
+ /* pass all pkts up to CPU */
+ .eth_dst = zero_mac,
+ .eth_dst_mask = zero_mac,
+ .acl = true,
+ },
};
static int rocker_port_ctrl_vlan_acl(struct rocker_port *rocker_port,
@@ -3787,11 +3806,14 @@ static int rocker_port_stp_update(struct rocker_port *rocker_port,
break;
case BR_STATE_LEARNING:
case BR_STATE_FORWARDING:
- want[ROCKER_CTRL_LINK_LOCAL_MCAST] = true;
+ if (!rocker_port_is_ovs(rocker_port))
+ want[ROCKER_CTRL_LINK_LOCAL_MCAST] = true;
want[ROCKER_CTRL_IPV4_MCAST] = true;
want[ROCKER_CTRL_IPV6_MCAST] = true;
if (rocker_port_is_bridged(rocker_port))
want[ROCKER_CTRL_DFLT_BRIDGING] = true;
+ else if (rocker_port_is_ovs(rocker_port))
+ want[ROCKER_CTRL_DFLT_OVS] = true;
else
want[ROCKER_CTRL_LOCAL_ARP] = true;
break;
@@ -5251,6 +5273,22 @@ static int rocker_port_bridge_leave(struct rocker_port *rocker_port)
return err;
}
+
+static int rocker_port_ovs_changed(struct rocker_port *rocker_port,
+ struct net_device *master)
+{
+ int err;
+
+ rocker_port->bridge_dev = master;
+
+ err = rocker_port_fwd_disable(rocker_port, SWITCHDEV_TRANS_NONE, 0);
+ if (err)
+ return err;
+ err = rocker_port_fwd_enable(rocker_port, SWITCHDEV_TRANS_NONE, 0);
+
+ return err;
+}
+
static int rocker_port_master_changed(struct net_device *dev)
{
struct rocker_port *rocker_port = netdev_priv(dev);
@@ -5263,11 +5301,16 @@ static int rocker_port_master_changed(struct net_device *dev)
* 3. Other, e.g. being added to or removed from a bond or openvswitch,
* in which case nothing is done
*/
- if (master && master->rtnl_link_ops &&
- !strcmp(master->rtnl_link_ops->kind, "bridge"))
- err = rocker_port_bridge_join(rocker_port, master);
- else if (rocker_port_is_bridged(rocker_port))
+ if (master && master->rtnl_link_ops) {
+ if (!strcmp(master->rtnl_link_ops->kind, "bridge"))
+ err = rocker_port_bridge_join(rocker_port, master);
+ else if (!strcmp(master->rtnl_link_ops->kind, "openvswitch"))
+ err = rocker_port_ovs_changed(rocker_port, master);
+ } else if (rocker_port_is_bridged(rocker_port)) {
err = rocker_port_bridge_leave(rocker_port);
+ } else if (rocker_port_is_ovs(rocker_port)) {
+ err = rocker_port_ovs_changed(rocker_port, NULL);
+ }
return err;
}
--
2.1.4
--
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