[PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Chas Williams]

If the bridge is allowing multiple VLANs, some VLANs may have
different MTUs.  Instead of choosing the minimum MTU for the
bridge interface, choose the maximum MTU of the bridge members.
With this the user only needs to set a larger MTU on the member
ports that are participating in the large MTU VLANS.

Signed-off-by: Chas Williams <3chas3@gmail.com>
---
 net/bridge/br.c         |  2 +-
 net/bridge/br_device.c  |  2 +-
 net/bridge/br_if.c      | 26 ++++++++++++++++++++++----
 net/bridge/br_private.h |  2 +-
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br.c b/net/bridge/br.c
index 7770481a6506..a3f95ab9d6a3 100644
--- a/net/bridge/br.c
+++ b/net/bridge/br.c
@@ -52,7 +52,7 @@ static int br_device_event(struct notifier_block *unused, unsigned long event, v
 
 	switch (event) {
 	case NETDEV_CHANGEMTU:
-		dev_set_mtu(br->dev, br_min_mtu(br));
+		dev_set_mtu(br->dev, br_mtu(br));
 		break;
 
 	case NETDEV_CHANGEADDR:
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 1285ca30ab0a..278fc999d355 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -224,7 +224,7 @@ static void br_get_stats64(struct net_device *dev,
 static int br_change_mtu(struct net_device *dev, int new_mtu)
 {
 	struct net_bridge *br = netdev_priv(dev);
-	if (new_mtu > br_min_mtu(br))
+	if (new_mtu > br_mtu(br))
 		return -EINVAL;
 
 	dev->mtu = new_mtu;
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 9ba4ed65c52b..48dc4d2e2be3 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -424,8 +424,18 @@ int br_del_bridge(struct net *net, const char *name)
 	return ret;
 }
 
+static bool min_mtu(int a, int b)
+{
+	return a < b ? 1 : 0;
+}
+
+static bool max_mtu(int a, int b)
+{
+	return a > b ? 1 : 0;
+}
+
 /* MTU of the bridge pseudo-device: ETH_DATA_LEN or the minimum of the ports */
-int br_min_mtu(const struct net_bridge *br)
+static int __br_mtu(const struct net_bridge *br, bool (compare_fn)(int, int))
 {
 	const struct net_bridge_port *p;
 	int mtu = 0;
@@ -436,13 +446,21 @@ int br_min_mtu(const struct net_bridge *br)
 		mtu = ETH_DATA_LEN;
 	else {
 		list_for_each_entry(p, &br->port_list, list) {
-			if (!mtu  || p->dev->mtu < mtu)
+			if (!mtu || compare_fn(p->dev->mtu, mtu))
 				mtu = p->dev->mtu;
 		}
 	}
 	return mtu;
 }
 
+int br_mtu(const struct net_bridge *br)
+{
+	if (br->vlan_enabled)
+		return __br_mtu(br, max_mtu);
+	else
+		return __br_mtu(br, min_mtu);
+}
+
 static void br_set_gso_limits(struct net_bridge *br)
 {
 	unsigned int gso_max_size = GSO_MAX_SIZE;
@@ -594,7 +612,7 @@ int br_add_if(struct net_bridge *br, struct net_device *dev,
 	if (changed_addr)
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, br->dev);
 
-	dev_set_mtu(br->dev, br_min_mtu(br));
+	dev_set_mtu(br->dev, br_mtu(br));
 	br_set_gso_limits(br);
 
 	kobject_uevent(&p->kobj, KOBJ_ADD);
@@ -641,7 +659,7 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
 	 */
 	del_nbp(p);
 
-	dev_set_mtu(br->dev, br_min_mtu(br));
+	dev_set_mtu(br->dev, br_mtu(br));
 	br_set_gso_limits(br);
 
 	spin_lock_bh(&br->lock);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8e13a64d8c99..048d5b51813b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -578,7 +578,7 @@ int br_del_bridge(struct net *net, const char *name);
 int br_add_if(struct net_bridge *br, struct net_device *dev,
 	      struct netlink_ext_ack *extack);
 int br_del_if(struct net_bridge *br, struct net_device *dev);
-int br_min_mtu(const struct net_bridge *br);
+int br_mtu(const struct net_bridge *br);
 netdev_features_t br_features_recompute(struct net_bridge *br,
 					netdev_features_t features);
 void br_port_flags_change(struct net_bridge_port *port, unsigned long mask);
-- 
2.13.6

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Nikolay Aleksandrov]

On 22/03/18 17:34, Chas Williams wrote:
> If the bridge is allowing multiple VLANs, some VLANs may have
> different MTUs.  Instead of choosing the minimum MTU for the
> bridge interface, choose the maximum MTU of the bridge members.
> With this the user only needs to set a larger MTU on the member
> ports that are participating in the large MTU VLANS.
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>
> ---
>  net/bridge/br.c         |  2 +-
>  net/bridge/br_device.c  |  2 +-
>  net/bridge/br_if.c      | 26 ++++++++++++++++++++++----
>  net/bridge/br_private.h |  2 +-
>  4 files changed, 25 insertions(+), 7 deletions(-)
> 

Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Roopa Prabhu]

On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
> If the bridge is allowing multiple VLANs, some VLANs may have
> different MTUs.  Instead of choosing the minimum MTU for the
> bridge interface, choose the maximum MTU of the bridge members.
> With this the user only needs to set a larger MTU on the member
> ports that are participating in the large MTU VLANS.
>
> Signed-off-by: Chas Williams <3chas3@gmail.com>
> ---

Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>

This or an equivalent fix is necessary: as stated above, today the
bridge mtu capped at min port mtu limits all
vlan devices on top of the vlan filtering bridge to min port mtu.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Roopa Prabhu]

On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>> If the bridge is allowing multiple VLANs, some VLANs may have
>> different MTUs.  Instead of choosing the minimum MTU for the
>> bridge interface, choose the maximum MTU of the bridge members.
>> With this the user only needs to set a larger MTU on the member
>> ports that are participating in the large MTU VLANS.
>>
>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>> ---
>
> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This or an equivalent fix is necessary: as stated above, today the
> bridge mtu capped at min port mtu limits all
> vlan devices on top of the vlan filtering bridge to min port mtu.


On further thought, since this patch changes default behavior, it may
upset people. ie with this patch, a vlan device
on the bridge by default will now use the  bridge max mtu and that
could cause unexpected drops in the bridge driver
if the xmit port had a lower mtu. This may surprise users.

The other equivalent fix i was thinking about is to keep the default
behavior as is, and allow a max mtu to be
configured on the bridge. This will allow a sys admin to fix the
current mtu limitations if
deployments require it.

we will submit an incremental patch to re-work this patch to restore
default behavior.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Toshiaki Makita]

On 2018/03/30 1:49, Roopa Prabhu wrote:
> On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>> different MTUs.  Instead of choosing the minimum MTU for the
>>> bridge interface, choose the maximum MTU of the bridge members.
>>> With this the user only needs to set a larger MTU on the member
>>> ports that are participating in the large MTU VLANS.
>>>
>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>> ---
>>
>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This or an equivalent fix is necessary: as stated above, today the
>> bridge mtu capped at min port mtu limits all
>> vlan devices on top of the vlan filtering bridge to min port mtu.
> 
> 
> On further thought, since this patch changes default behavior, it may
> upset people. ie with this patch, a vlan device
> on the bridge by default will now use the  bridge max mtu and that
> could cause unexpected drops in the bridge driver
> if the xmit port had a lower mtu. This may surprise users.
> 
> The other equivalent fix i was thinking about is to keep the default
> behavior as is, and allow a max mtu to be
> configured on the bridge. This will allow a sys admin to fix the
> current mtu limitations if
> deployments require it.
> 
> we will submit an incremental patch to re-work this patch to restore
> default behavior.

+1

This makes sense to me.

-- 
Toshiaki Makita

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Chas Williams]

On Thu, Mar 29, 2018 at 9:02 PM, Toshiaki Makita
<makita.toshiaki@lab.ntt.co.jp> wrote:
> On 2018/03/30 1:49, Roopa Prabhu wrote:
>> On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>>> different MTUs.  Instead of choosing the minimum MTU for the
>>>> bridge interface, choose the maximum MTU of the bridge members.
>>>> With this the user only needs to set a larger MTU on the member
>>>> ports that are participating in the large MTU VLANS.
>>>>
>>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>>> ---
>>>
>>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This or an equivalent fix is necessary: as stated above, today the
>>> bridge mtu capped at min port mtu limits all
>>> vlan devices on top of the vlan filtering bridge to min port mtu.
>>
>>
>> On further thought, since this patch changes default behavior, it may
>> upset people. ie with this patch, a vlan device
>> on the bridge by default will now use the  bridge max mtu and that
>> could cause unexpected drops in the bridge driver
>> if the xmit port had a lower mtu. This may surprise users.

It only changes the default behavior when you are using VLAN aware bridges.
The behavior remains the same otherwise.  I don't know if VLAN aware bridges
are that popular yet so there probably isn't any particular
expectation from those
bridges.

I don't think those drops are unexpected.  If a user has misconfigured
the bridge
we can't be expected to fix that for them.  It is the user's
responsbility to ensure
that the ports on the VLAN have a size consistent with the traffic
they expect to
pass.

>>
>> The other equivalent fix i was thinking about is to keep the default
>> behavior as is, and allow a max mtu to be
>> configured on the bridge. This will allow a sys admin to fix the
>> current mtu limitations if
>> deployments require it.
>>
>> we will submit an incremental patch to re-work this patch to restore
>> default behavior.
>
> +1
>
> This makes sense to me.
>
> --
> Toshiaki Makita
>

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Roopa Prabhu]

On Fri, Mar 30, 2018 at 12:54 PM, Chas Williams <3chas3@gmail.com> wrote:
> On Thu, Mar 29, 2018 at 9:02 PM, Toshiaki Makita
> <makita.toshiaki@lab.ntt.co.jp> wrote:
>> On 2018/03/30 1:49, Roopa Prabhu wrote:
>>> On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>>>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>>>> different MTUs.  Instead of choosing the minimum MTU for the
>>>>> bridge interface, choose the maximum MTU of the bridge members.
>>>>> With this the user only needs to set a larger MTU on the member
>>>>> ports that are participating in the large MTU VLANS.
>>>>>
>>>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>>>> ---
>>>>
>>>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This or an equivalent fix is necessary: as stated above, today the
>>>> bridge mtu capped at min port mtu limits all
>>>> vlan devices on top of the vlan filtering bridge to min port mtu.
>>>
>>>
>>> On further thought, since this patch changes default behavior, it may
>>> upset people. ie with this patch, a vlan device
>>> on the bridge by default will now use the  bridge max mtu and that
>>> could cause unexpected drops in the bridge driver
>>> if the xmit port had a lower mtu. This may surprise users.
>
> It only changes the default behavior when you are using VLAN aware bridges.
> The behavior remains the same otherwise.  I don't know if VLAN aware bridges
> are that popular yet so there probably isn't any particular
> expectation from those
> bridges.

they are popular...in-fact they are the default bridge mode on our
network switches.
And they have been around for some time now to ignore its users.
Plus it is not right to change default mtu behavior for one mode of the bridge
and not the others (bridge mtu handling from user-space is complex enough today
due to dynamic mtu changes on port enslave/deslave).


>
> I don't think those drops are unexpected.  If a user has misconfigured
> the bridge
> we can't be expected to fix that for them.  It is the user's
> responsbility to ensure
> that the ports on the VLAN have a size consistent with the traffic
> they expect to
> pass.
>

By default they are not expected today. The problem is changing the bridge
to max mtu changes 'all' the vlan devices on top of the vlan aware bridge to
max mtu by default which makes drops at the bridge driver more common if the
user had mixed mtu on its ports.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Chas Williams]

On Mon, Apr 2, 2018 at 11:08 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On Fri, Mar 30, 2018 at 12:54 PM, Chas Williams <3chas3@gmail.com> wrote:
>> On Thu, Mar 29, 2018 at 9:02 PM, Toshiaki Makita
>> <makita.toshiaki@lab.ntt.co.jp> wrote:
>>> On 2018/03/30 1:49, Roopa Prabhu wrote:
>>>> On Thu, Mar 22, 2018 at 9:53 PM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>>> On Thu, Mar 22, 2018 at 8:34 AM, Chas Williams <3chas3@gmail.com> wrote:
>>>>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>>>>> different MTUs.  Instead of choosing the minimum MTU for the
>>>>>> bridge interface, choose the maximum MTU of the bridge members.
>>>>>> With this the user only needs to set a larger MTU on the member
>>>>>> ports that are participating in the large MTU VLANS.
>>>>>>
>>>>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>>>>> ---
>>>>>
>>>>> Acked-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>>
>>>>> This or an equivalent fix is necessary: as stated above, today the
>>>>> bridge mtu capped at min port mtu limits all
>>>>> vlan devices on top of the vlan filtering bridge to min port mtu.
>>>>
>>>>
>>>> On further thought, since this patch changes default behavior, it may
>>>> upset people. ie with this patch, a vlan device
>>>> on the bridge by default will now use the  bridge max mtu and that
>>>> could cause unexpected drops in the bridge driver
>>>> if the xmit port had a lower mtu. This may surprise users.
>>
>> It only changes the default behavior when you are using VLAN aware bridges.
>> The behavior remains the same otherwise.  I don't know if VLAN aware bridges
>> are that popular yet so there probably isn't any particular
>> expectation from those
>> bridges.
>
> they are popular...in-fact they are the default bridge mode on our
> network switches.
> And they have been around for some time now to ignore its users.
> Plus it is not right to change default mtu behavior for one mode of the bridge
> and not the others (bridge mtu handling from user-space is complex enough today
> due to dynamic mtu changes on port enslave/deslave).

I don't see the issue with one mode of bridge behaving differently
from another mode.
The VLAN behavior between the two bridge modes is completely different so having
a different MTU behavior doesn't seem that surprising.

You are potentially mixing different sized VLAN on a same bridge.  The only sane
choice is to pick the largest MTU for the bridge.  This lets you have
whatever MTU
is appropriate on the child VLAN interfaces of the bridge.  If you
attempt to forward
from a port with a larger MTU to a smaller MTU, you get the expected behavior.

Forcing the end user to configure all the ports to the maximum MTU of
all the VLANs
on the bridge is wrong IMHO.  You then risk attempting to forward
oversize packets
on a network that can't support that.

>
>>
>> I don't think those drops are unexpected.  If a user has misconfigured
>> the bridge
>> we can't be expected to fix that for them.  It is the user's
>> responsbility to ensure
>> that the ports on the VLAN have a size consistent with the traffic
>> they expect to
>> pass.
>>
>
> By default they are not expected today. The problem is changing the bridge
> to max mtu changes 'all' the vlan devices on top of the vlan aware bridge to
> max mtu by default which makes drops at the bridge driver more common if the
> user had mixed mtu on its ports.

That's not been my experience.  The MTU on the vlan devices is only
limited by the
bridges's MTU.  Setting the bridge MTU doesn't change the children
VLAN devices MTUs.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Roopa Prabhu]

On Mon, Apr 2, 2018 at 8:26 AM, Chas Williams <3chas3@gmail.com> wrote:
> On Mon, Apr 2, 2018 at 11:08 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>

[snip]

>> they are popular...in-fact they are the default bridge mode on our
>> network switches.
>> And they have been around for some time now to ignore its users.
>> Plus it is not right to change default mtu behavior for one mode of the bridge
>> and not the others (bridge mtu handling from user-space is complex enough today
>> due to dynamic mtu changes on port enslave/deslave).
>
> I don't see the issue with one mode of bridge behaving differently
> from another mode.
> The VLAN behavior between the two bridge modes is completely different so having
> a different MTU behavior doesn't seem that surprising.
>
> You are potentially mixing different sized VLAN on a same bridge.  The only sane
> choice is to pick the largest MTU for the bridge.  This lets you have
> whatever MTU
> is appropriate on the child VLAN interfaces of the bridge.  If you
> attempt to forward
> from a port with a larger MTU to a smaller MTU, you get the expected behavior.


you mean larger MTU on the vlan device on the bridge to a smaller MTU
on the bridge port ?.
this will result in dropping the packet. how is this supposed to be
expected default behavior ?.


>
> Forcing the end user to configure all the ports to the maximum MTU of
> all the VLANs
> on the bridge is wrong IMHO.
> You then risk attempting to forward
> oversize packets
> on a network that can't support that.

I am a bit confused: Are you trying to solve the config problem by
implicitly making it the default and there by creating the oversize
packet drop issue by default ?


>
>>
>>>
>>> I don't think those drops are unexpected.  If a user has misconfigured
>>> the bridge
>>> we can't be expected to fix that for them.  It is the user's
>>> responsbility to ensure
>>> that the ports on the VLAN have a size consistent with the traffic
>>> they expect to
>>> pass.
>>>
>>
>> By default they are not expected today. The problem is changing the bridge
>> to max mtu changes 'all' the vlan devices on top of the vlan aware bridge to
>> max mtu by default which makes drops at the bridge driver more common if the
>> user had mixed mtu on its ports.
>
> That's not been my experience.  The MTU on the vlan devices is only
> limited by the
> bridges's MTU.  Setting the bridge MTU doesn't change the children
> VLAN devices MTUs.

It does not, but it now allows vlan devices on the bridge to have a
larger MTU if they need to (some or all of them).
This is consistent with vxlan driver as well: picks default mtu to be
lower or equal to the default dst dev mtu and allows user to override
it with a larger MTU.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Chas Williams]

On Tue, Apr 3, 2018 at 2:13 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On Mon, Apr 2, 2018 at 8:26 AM, Chas Williams <3chas3@gmail.com> wrote:
>> On Mon, Apr 2, 2018 at 11:08 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>>>
>
> [snip]
>
>>> they are popular...in-fact they are the default bridge mode on our
>>> network switches.
>>> And they have been around for some time now to ignore its users.
>>> Plus it is not right to change default mtu behavior for one mode of the bridge
>>> and not the others (bridge mtu handling from user-space is complex enough today
>>> due to dynamic mtu changes on port enslave/deslave).
>>
>> I don't see the issue with one mode of bridge behaving differently
>> from another mode.
>> The VLAN behavior between the two bridge modes is completely different so having
>> a different MTU behavior doesn't seem that surprising.
>>
>> You are potentially mixing different sized VLAN on a same bridge.  The only sane
>> choice is to pick the largest MTU for the bridge.  This lets you have
>> whatever MTU
>> is appropriate on the child VLAN interfaces of the bridge.  If you
>> attempt to forward
>> from a port with a larger MTU to a smaller MTU, you get the expected behavior.
>
>
> you mean larger MTU on the vlan device on the bridge to a smaller MTU
> on the bridge port ?.
> this will result in dropping the packet. how is this supposed to be
> expected default behavior ?.

If a user configures the VLAN device to be a larger than MTU than the port,
then yes, I expect the packet to be dropped.  That's a msconfiguration of either
the VLAN's or port's MTU.  We can't protect the user from that by simply making
sure they can't mismatch the MTUs because you can still get packets dropped
during ingress from the large MTU VLAN.

>> Forcing the end user to configure all the ports to the maximum MTU of
>> all the VLANs
>> on the bridge is wrong IMHO.
>> You then risk attempting to forward
>> oversize packets
>> on a network that can't support that.
>
> I am a bit confused: Are you trying to solve the config problem by
> implicitly making it the default and there by creating the oversize
> packet drop issue by default ?

I am attempting to allow a configuration that lets me choose the appropriate
MTU size for each port.  With the previous code to configure a VLAN device
with an MTU of 9000, I would need to configure all the ports
on the bridge with an MTU of 9000 regardless of whether those ports should
be passing large MTU traffic.  I am creating a potential packet drop issue
by forwarding traffic between ports that have an artificially inflated MTUs.


>>>> I don't think those drops are unexpected.  If a user has misconfigured
>>>> the bridge
>>>> we can't be expected to fix that for them.  It is the user's
>>>> responsbility to ensure
>>>> that the ports on the VLAN have a size consistent with the traffic
>>>> they expect to
>>>> pass.
>>>>
>>>
>>> By default they are not expected today. The problem is changing the bridge
>>> to max mtu changes 'all' the vlan devices on top of the vlan aware bridge to
>>> max mtu by default which makes drops at the bridge driver more common if the
>>> user had mixed mtu on its ports.
>>
>> That's not been my experience.  The MTU on the vlan devices is only
>> limited by the
>> bridges's MTU.  Setting the bridge MTU doesn't change the children
>> VLAN devices MTUs.
>
> It does not, but it now allows vlan devices on the bridge to have a
> larger MTU if they need to (some or all of them).
> This is consistent with vxlan driver as well: picks default mtu to be
> lower or equal to the default dst dev mtu and allows user to override
> it with a larger MTU.

The VLAN device MTU can't be larger than the parent MTU.  The end user
is just going to set the parent bridge MTU to be larger anyway, so why not
just make that the default?

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[David Miller]

From: Chas Williams <3chas3@gmail.com>
Date: Thu, 22 Mar 2018 11:34:06 -0400

> If the bridge is allowing multiple VLANs, some VLANs may have
> different MTUs.  Instead of choosing the minimum MTU for the
> bridge interface, choose the maximum MTU of the bridge members.
> With this the user only needs to set a larger MTU on the member
> ports that are participating in the large MTU VLANS.
> 
> Signed-off-by: Chas Williams <3chas3@gmail.com>

Applied, thanks.

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Nikolay Aleksandrov]

On 23/03/18 18:17, David Miller wrote:
> From: Chas Williams <3chas3@gmail.com>
> Date: Thu, 22 Mar 2018 11:34:06 -0400
> 
>> If the bridge is allowing multiple VLANs, some VLANs may have
>> different MTUs.  Instead of choosing the minimum MTU for the
>> bridge interface, choose the maximum MTU of the bridge members.
>> With this the user only needs to set a larger MTU on the member
>> ports that are participating in the large MTU VLANS.
>>
>> Signed-off-by: Chas Williams <3chas3@gmail.com>
> 
> Applied, thanks.
> 

Argh, this will break on builds without vlans because br->vlan_enabled shouldn't
be accessed directly. I missed that when reviewing.
I'll send a follow up fix in a second that uses br_vlan_enabled().

Re: [PATCH net-next] bridge: Allow max MTU when multiple VLANs present

[Chas Williams]

On Fri, Mar 23, 2018 at 12:20 PM, Nikolay Aleksandrov
<nikolay@cumulusnetworks.com> wrote:
> On 23/03/18 18:17, David Miller wrote:
>> From: Chas Williams <3chas3@gmail.com>
>> Date: Thu, 22 Mar 2018 11:34:06 -0400
>>
>>> If the bridge is allowing multiple VLANs, some VLANs may have
>>> different MTUs.  Instead of choosing the minimum MTU for the
>>> bridge interface, choose the maximum MTU of the bridge members.
>>> With this the user only needs to set a larger MTU on the member
>>> ports that are participating in the large MTU VLANS.
>>>
>>> Signed-off-by: Chas Williams <3chas3@gmail.com>
>>
>> Applied, thanks.
>>
>
> Argh, this will break on builds without vlans because br->vlan_enabled shouldn't
> be accessed directly. I missed that when reviewing.
> I'll send a follow up fix in a second that uses br_vlan_enabled().
>

Sorry I missed that one.  Thanks for covering!

[PATCH net-next] net: bridge: fix direct access to bridge vlan_enabled and use helper

[Nikolay Aleksandrov]

We need to use br_vlan_enabled() helper otherwise we'll break builds
without bridge vlans:
net/bridge//br_if.c: In function ‘br_mtu’:
net/bridge//br_if.c:458:8: error: ‘const struct net_bridge’ has no
member named ‘vlan_enabled’
  if (br->vlan_enabled)
        ^
net/bridge//br_if.c:462:1: warning: control reaches end of non-void
function [-Wreturn-type]
 }
 ^
scripts/Makefile.build:324: recipe for target 'net/bridge//br_if.o'
failed

Fixes: 419d14af9e07 ("bridge: Allow max MTU when multiple VLANs present")
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 net/bridge/br_if.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index 48dc4d2e2be3..87b2afd455c7 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -455,7 +455,7 @@ static int __br_mtu(const struct net_bridge *br, bool (compare_fn)(int, int))
 
 int br_mtu(const struct net_bridge *br)
 {
-	if (br->vlan_enabled)
+	if (br_vlan_enabled(br->dev))
 		return __br_mtu(br, max_mtu);
 	else
 		return __br_mtu(br, min_mtu);
-- 
2.1.4

Re: [PATCH net-next] net: bridge: fix direct access to bridge vlan_enabled and use helper

[David Miller]

From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Fri, 23 Mar 2018 18:27:06 +0200

> We need to use br_vlan_enabled() helper otherwise we'll break builds
> without bridge vlans:
> net/bridge//br_if.c: In function ‘br_mtu’:
> net/bridge//br_if.c:458:8: error: ‘const struct net_bridge’ has no
> member named ‘vlan_enabled’
>   if (br->vlan_enabled)
>         ^
> net/bridge//br_if.c:462:1: warning: control reaches end of non-void
> function [-Wreturn-type]
>  }
>  ^
> scripts/Makefile.build:324: recipe for target 'net/bridge//br_if.o'
> failed
> 
> Fixes: 419d14af9e07 ("bridge: Allow max MTU when multiple VLANs present")
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

Applied, thanks for responding to this issue so quickly.