Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bridge: remove default pvid for veth interface #667

Closed
bephinix opened this issue Sep 26, 2021 · 1 comment · Fixed by #875
Closed

bridge: remove default pvid for veth interface #667

bephinix opened this issue Sep 26, 2021 · 1 comment · Fixed by #875

Comments

@bephinix
Copy link

Problem

Using the bridge plugin will create a seperate veth-pair per pod with one end of the veth-pair connected to the specified/configured bridge.
If one uses VLANs with the bridge plugin and a pod is configured to use the bridge network with e.g. VLAN 123, the veth-pair endpoint connected to the bridge has VLAN 123 and 1 configured:

port  vlan-id
br0   1 Egress Untagged
      123 PVID Egress Untagged

Idea/Solution

VLAN 1 is the DefaultPVID for a bridge or connected device, thus it is automatically configured.
Whereas the bridge (which has to be created and maintained manually) can be modified to not accept VLAN 1 traffic (e.g. by using ip link ... vlan_default_pvid 0 or systemd option DefaultPVID=none), this should also be possible for the veth-pair endpoint.

As current configurations should not be affected, one should add an option (e.g. nodefaultpvid) which will trigger the removal of PVID 1:

if vlanID != 0 {
    err = netlink.BridgeVlanDel(hostVeth, uint16(1), true, true, false, true)
    if err != nil {
        return nil, nil, fmt.Errorf("failed to delete default vlan tag on interface %q: %v", hostIface.Name, err)
    }
    err = netlink.BridgeVlanAdd(hostVeth, uint16(vlanID), true, true, false, true)
    if err != nil {
        return nil, nil, fmt.Errorf("failed to setup vlan tag on interface %q: %v", hostIface.Name, err)
    }
}

The proposed modification will also work if one wants to use VLAN 1 for a pod.

(Ref.: https://github.com/containernetworking/plugins/blob/master/plugins/main/bridge/bridge.go#L366)

@github-actions github-actions bot added the Stale label Mar 17, 2022
@dcbw dcbw reopened this Mar 21, 2023
phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 21, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
@dcbw
Copy link
Member

dcbw commented Mar 21, 2023

Seems like it may deserve a knob to prevent the untagged vlan1 behavior. I'd think the default expectation is that if you ask for a specific VLAN, you get that and only that VLAN, not that plus other stuff.

phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 21, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
@github-actions github-actions bot removed the Stale label Mar 22, 2023
phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 23, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 23, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 23, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
phoracek added a commit to phoracek/containernetworking-plugins that referenced this issue Mar 23, 2023
The bridge CNI allows users to select a VLAN to which a Pod should be
connected:

  {
    "cniVersion": "0.3.1",
    "name": "mynet",
    "type": "bridge",
    "bridge": "mynet0",
    "vlan": 100
  }

The expected behavior in such a case is that the Pod is isolated within
the requested VLAN.

That however is not the case. Today, the underlying OS applies the
default VLAN 1 to the bridge itself and to every veth connected to it.
Due to that, the Pod connected to a VLAN is still receiving all the
native traffic of the bridge.

That can be reproduced by:
* Using the configuration listed above,
* setting IP 11.11.11.1/24 on the bridge,
* setting IP 11.11.11.10/24 in the Pod,
* broadcasting ARP on the bridge: arping -A -I mynet0 11.11.11.1
* listening to ARP in the Pod: tcpdump -lni net1 arp

With this suggested fix, the implicitly added VLAN 1 is removed in case
another VLAN ID is provided.

Fixes: containernetworking#667

Signed-off-by: Petr Horacek <hrck@protonmail.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 23, 2023
These changes prevent pods connected to a VLAN
to receive the native traffic of the bridge.

Fixes: containernetworking#667

Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 23, 2023
These changes prevent pods connected to a VLAN
to receive the native traffic of the bridge.

Fixes: containernetworking#667

Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 31, 2023
These changes prevent pods connected to a VLAN
to receive the native traffic of the bridge.

Fixes: containernetworking#667

Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 31, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 31, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 31, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Mar 31, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan.

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 3, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 4, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
mlguerrero12 added a commit to mlguerrero12/plugins that referenced this issue Apr 5, 2023
This new parameter allows users to remove the default vlan

Fixes: containernetworking#667
Signed-off-by: Marcelo Guerrero Viveros <marguerr@redhat.com>
phoracek added a commit to phoracek/cluster-network-addons-operator that referenced this issue Apr 19, 2023
This introduces bug fixes resolving issues with VLAN 1 leakage and
performance issues of MAC spoof filtering [2].

[1] containernetworking/plugins#667
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2173485

Signed-off-by: Petr Horacek <hrck@protonmail.com>
phoracek added a commit to phoracek/cluster-network-addons-operator that referenced this issue Apr 19, 2023
This introduces bug fixes resolving issues with VLAN 1 leakage and
performance issues of MAC spoof filtering [2].

[1] containernetworking/plugins#667
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2173485

Signed-off-by: Petr Horacek <hrck@protonmail.com>
kubevirt-bot pushed a commit to kubevirt/cluster-network-addons-operator that referenced this issue Apr 19, 2023
This introduces bug fixes resolving issues with VLAN 1 leakage and
performance issues of MAC spoof filtering [2].

[1] containernetworking/plugins#667
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2173485

Signed-off-by: Petr Horacek <hrck@protonmail.com>
maiqueb pushed a commit to maiqueb/cluster-network-addons-operator that referenced this issue Apr 20, 2023
This introduces bug fixes resolving issues with VLAN 1 leakage and
performance issues of MAC spoof filtering [2].

[1] containernetworking/plugins#667
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2173485

Signed-off-by: Petr Horacek <hrck@protonmail.com>
(cherry picked from commit 755a53e)
kubevirt-bot pushed a commit to kubevirt/cluster-network-addons-operator that referenced this issue Apr 20, 2023
This introduces bug fixes resolving issues with VLAN 1 leakage and
performance issues of MAC spoof filtering [2].

[1] containernetworking/plugins#667
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2173485

Signed-off-by: Petr Horacek <hrck@protonmail.com>
(cherry picked from commit 755a53e)

Co-authored-by: Petr Horacek <hrck@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants