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.hor...@netronome.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.hor...@netronome.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.hor...@netronome.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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html