On 1/17/19 5:47 AM, Ido Schimmel wrote:
> On Wed, Jan 16, 2019 at 12:00:49PM -0800, Florian Fainelli wrote:
>> Some Ethernet switches might not be able to support disabling multicast
>> flooding globally when e.g: several bridges span the same physical
>> device, propagate the return value of br_mc_disabled_update() such that
>> this propagates correctly to user-space.
>>
>> Signed-off-by: Florian Fainelli <f.faine...@gmail.com>
>> ---
>> net/bridge/br_multicast.c | 19 ++++++++++++++-----
>> 1 file changed, 14 insertions(+), 5 deletions(-)
>>
>> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
>> index 3aeff0895669..09fc92541873 100644
>> --- a/net/bridge/br_multicast.c
>> +++ b/net/bridge/br_multicast.c
>> @@ -813,7 +813,7 @@ static void br_ip6_multicast_port_query_expired(struct
>> timer_list *t)
>> }
>> #endif
>>
>> -static void br_mc_disabled_update(struct net_device *dev, bool value)
>> +static int br_mc_disabled_update(struct net_device *dev, bool value)
>> {
>> struct switchdev_attr attr = {
>> .orig_dev = dev,
>> @@ -822,11 +822,13 @@ static void br_mc_disabled_update(struct net_device
>> *dev, bool value)
>> .u.mc_disabled = !value,
>> };
>>
>> - switchdev_port_attr_set(dev, &attr);
>> + return switchdev_port_attr_set(dev, &attr);
>
> Did you test this for veth? If switchdev ops are not defined, this
> returns -EOPNOTSUPP. See __br_vlan_filter_toggle() for reference
I did not, good catch, I will add a check on -EOPNOTSUPP and silence
that error code:
if (err && err != -ENOTSUPP)
return err;
>
>> }
>>
>> int br_multicast_add_port(struct net_bridge_port *port)
>> {
>> + int ret;
>> +
>> port->multicast_router = MDB_RTR_TYPE_TEMP_QUERY;
>>
>> timer_setup(&port->multicast_router_timer,
>> @@ -837,8 +839,11 @@ int br_multicast_add_port(struct net_bridge_port *port)
>> timer_setup(&port->ip6_own_query.timer,
>> br_ip6_multicast_port_query_expired, 0);
>> #endif
>> - br_mc_disabled_update(port->dev,
>> - br_opt_get(port->br, BROPT_MULTICAST_ENABLED));
>> + ret = br_mc_disabled_update(port->dev,
>> + br_opt_get(port->br,
>> + BROPT_MULTICAST_ENABLED));
>> + if (ret)
>> + return ret;
>>
>> port->mcast_stats = netdev_alloc_pcpu_stats(struct bridge_mcast_stats);
>> if (!port->mcast_stats)
>> @@ -1937,12 +1942,16 @@ static void br_multicast_start_querier(struct
>> net_bridge *br,
>> int br_multicast_toggle(struct net_bridge *br, unsigned long val)
>> {
>> struct net_bridge_port *port;
>> + int err;
>>
>> spin_lock_bh(&br->multicast_lock);
>> if (!!br_opt_get(br, BROPT_MULTICAST_ENABLED) == !!val)
>> goto unlock;
>>
>> - br_mc_disabled_update(br->dev, val);
>> + err = br_mc_disabled_update(br->dev, val);
>> + if (err)
>> + goto unlock;
>> +
>> br_opt_toggle(br, BROPT_MULTICAST_ENABLED, !!val);
>> if (!br_opt_get(br, BROPT_MULTICAST_ENABLED))
>> goto unlock;
>
> You never return the error
Huh, indeed, thanks!
--
Florian
