core: allow users to configure MTU for bridge and veth interfaces from config
#111
Conversation
…m config Netavark config accepts `MTU` as network option so allow users to configure the same for both `bridge` and `veth` interfaces if defined. Signed-off-by: Aditya Rajan <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: flouthoc The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
I want to rebase this PR after: #110 |
| @@ -36,6 +36,23 @@ impl Core { | |||
| let mut response_net_addresses: Vec<NetAddress> = Vec::new(); | |||
| // interfaces map, but we only ever expect one, for response | |||
| let mut interfaces: HashMap<String, types::NetInterface> = HashMap::new(); | |||
| // mtu to configure, 0 means it was not set do nothing. | |||
There was a problem hiding this comment.
We should not set mtu to 0. Although kernel allows it but its a bug https://serverfault.com/a/652604
So netavark will NOOP in case of mtu: 0
There was a problem hiding this comment.
I agree 0 means default so we should do nothing
| @@ -750,6 +789,12 @@ impl CoreUtils { | |||
| } | |||
| } | |||
|
|
|||
| if mtu != 0 { | |||
There was a problem hiding this comment.
small nit, I prefer mtu > 0 syntax as it is not clear here if this is a unit or regular int in which case != would allow negative values.
Not a blocker
There was a problem hiding this comment.
Sure i'll fix this.
|
LGTM |
|
/lgtm |
|
whoops I really wanted to get this merged first #110 no issues i'll rebase the other one. |
Netavark config accepts `MTU` as network option so allow users to configure the same for `macvlan` interface. See https://github.com/containernetworking/plugins/blob/master/plugins/main/macvlan/macvlan.go#L181 Follow up PR to: containers#111 Signed-off-by: Aditya Rajan <[email protected]>
Netavark config accepts
MTUas network option so allow users toconfigure the same for both
bridgeandvethinterfaces if defined.Closes: #107