[PATCH] network: explicitly set the MTU of the bridge device.

Laine Stump posted 1 patch 3 years, 3 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210114171649.118057-1-laine@redhat.com
src/network/bridge_driver.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] network: explicitly set the MTU of the bridge device.
Posted by Laine Stump 3 years, 3 months ago
In the past, the MTU of libvirt virtual network bridge devices was
implicitly set by setting the MTU of the "dummy tap device" (which was
being added in order to force a particular MAC address from the
bridge). But the dummy tap device was removed in commit ee6c936fbb
(libvirt-6.8.0), and so the mtu setting in the network is ignored.

The solution is, of course, to explicitly set the bridge device MTU
when it is created.

Note that any guest interface with a larger MTU that is attached will
cause the bridge to (temporarily) assume the larger MTU, but it will
revert to the bridge's own MTU when that device is deleted (this is
not due to anything libvirt does; it's just how Linux host bridges
work).

Resolves: https://bugzilla.redhat.com/1913561
Signed-off-by: Laine Stump <laine@redhat.com>

    ee6c936fbbo-set-mtu

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/network/bridge_driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index b7c604eaea..519a473995 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -2336,6 +2336,9 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
 
     /* Set bridge options */
 
+    if (def->mtu && virNetDevSetMTU(def->bridge, def->mtu) < 0)
+        goto error;
+
     /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay
      * expects milliseconds
      */
-- 
2.29.2

Re: [PATCH] network: explicitly set the MTU of the bridge device.
Posted by Ján Tomko 3 years, 3 months ago
There is no need fo the period at the end of the commit summary.

On a Thursday in 2021, Laine Stump wrote:
>In the past, the MTU of libvirt virtual network bridge devices was
>implicitly set by setting the MTU of the "dummy tap device" (which was
>being added in order to force a particular MAC address from the
>bridge). But the dummy tap device was removed in commit ee6c936fbb
>(libvirt-6.8.0), and so the mtu setting in the network is ignored.
>
>The solution is, of course, to explicitly set the bridge device MTU
>when it is created.
>
>Note that any guest interface with a larger MTU that is attached will
>cause the bridge to (temporarily) assume the larger MTU, but it will
>revert to the bridge's own MTU when that device is deleted (this is
>not due to anything libvirt does; it's just how Linux host bridges
>work).
>
>Resolves: https://bugzilla.redhat.com/1913561
>Signed-off-by: Laine Stump <laine@redhat.com>
>
>    ee6c936fbbo-set-mtu
>

This looks like a name of a branch. If you want to preserve the commit
this improves upon, I have been using the following format:

Fixes: ee6c936fbbfb217175326f0201d59cc6727a0678

>Signed-off-by: Laine Stump <laine@redhat.com>
>---
> src/network/bridge_driver.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>index b7c604eaea..519a473995 100644
>--- a/src/network/bridge_driver.c
>+++ b/src/network/bridge_driver.c
>@@ -2336,6 +2336,9 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
>
>     /* Set bridge options */
>
>+    if (def->mtu && virNetDevSetMTU(def->bridge, def->mtu) < 0)
>+        goto error;
>+
>     /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay
>      * expects milliseconds
>      */

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano
Re: [PATCH] network: explicitly set the MTU of the bridge device.
Posted by Laine Stump 3 years, 3 months ago
On 1/15/21 2:51 AM, Ján Tomko wrote:
> There is no need fo the period at the end of the commit summary.
> 
> On a Thursday in 2021, Laine Stump wrote:
>> In the past, the MTU of libvirt virtual network bridge devices was
>> implicitly set by setting the MTU of the "dummy tap device" (which was
>> being added in order to force a particular MAC address from the
>> bridge). But the dummy tap device was removed in commit ee6c936fbb
>> (libvirt-6.8.0), and so the mtu setting in the network is ignored.
>>
>> The solution is, of course, to explicitly set the bridge device MTU
>> when it is created.
>>
>> Note that any guest interface with a larger MTU that is attached will
>> cause the bridge to (temporarily) assume the larger MTU, but it will
>> revert to the bridge's own MTU when that device is deleted (this is
>> not due to anything libvirt does; it's just how Linux host bridges
>> work).
>>
>> Resolves: https://bugzilla.redhat.com/1913561
>> Signed-off-by: Laine Stump <laine@redhat.com>
>>
>>    ee6c936fbbo-set-mtu

Actually, I have absolutely 0 idea where that came from :-/. I only 
noticed it sitting at the end of the commit log message when I went back 
later to rebase.

Hmm. I guess my best guess is that it happened when I clicked my middle 
mouse button to paste the commit id into the middle of the comment, but 
it pasted somewhere else in that big instructional comment section git 
puts at the end of the suggested log text, and there was apparently a 
newline in the text that got pasted, so it ended up turning a bit of the 
commented text into a non-comment and it got tacked on the end of the 
log message.

Or something like that.


>>
> 
> This looks like a name of a branch. If you want to preserve the commit
> this improves upon, I have been using the following format:


> 
> Fixes: ee6c936fbbfb217175326f0201d59cc6727a0678

For some reason I hadn't thought to do that, but it does make sense. 
I'll add that before I push.

Thanks!

> 
>> Signed-off-by: Laine Stump <laine@redhat.com>
>> ---
>> src/network/bridge_driver.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index b7c604eaea..519a473995 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -2336,6 +2336,9 @@ 
>> networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
>>
>>     /* Set bridge options */
>>
>> +    if (def->mtu && virNetDevSetMTU(def->bridge, def->mtu) < 0)
>> +        goto error;
>> +
>>     /* delay is configured in seconds, but virNetDevBridgeSetSTPDelay
>>      * expects milliseconds
>>      */
> 
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> 
> Jano