On 8/26/19 5:38 AM, Andrew Lunn wrote: > On Mon, Aug 26, 2019 at 10:11:12AM +0200, Horatiu Vultur wrote: >> When a network port is added to a bridge then the port is added in >> promisc mode. Some HW that has bridge capabilities(can learn, forward, >> flood etc the frames) they are disabling promisc mode in the network >> driver when the port is added to the SW bridge. >> >> This patch adds the feature NETIF_F_HW_BR_CAP so that the network ports >> that have this feature will not be set in promisc mode when they are >> added to a SW bridge. >> >> In this way the HW that has bridge capabilities don't need to send all the >> traffic to the CPU and can also implement the promisc mode and toggle it >> using the command 'ip link set dev swp promisc on' > > Hi Horatiu > > I'm still not convinced this is needed. The model is, the hardware is > there to accelerate what Linux can do in software. Any peculiarities > of the accelerator should be hidden in the driver. If the accelerator > can do its job without needing promisc mode, do that in the driver. > > So you are trying to differentiate between promisc mode because the > interface is a member of a bridge, and promisc mode because some > application, like pcap, has asked for promisc mode. > > dev->promiscuity is a counter. So what you can do it look at its > value, and how the interface is being used. If the interface is not a > member of a bridge, and the count > 0, enable promisc mode in the > accelerator. If the interface is a member of a bridge, and the count > > 1, enable promisc mode in the accelerator. That is an excellent suggestion actually. Horatiu, the other issue with your approach here is that the features don't propagate to/from lower/upper/real devices, so if e.g.: you have a VLAN interface enslaved as a part of the bridge, or a bond, or a tunnel interface, the logic won't make us check NETIF_F_HW_BR_CAP because those virtual network devices won't inherit it from their real device. I am not suggesting you fix this with your patch series, but rather, seek a driver local solution. -- Florian
