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.

Sign up for GitHub

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

Add detailed info about vlan capabilities on bridge #134

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add detailed info about vlan capabilities on bridge #134

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 37 additions & 18 deletions content/plugins/current/main/bridge.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,22 +47,6 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa
}
```

## Example L2-only vlan configuration
```json
{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "bridge",
"bridge": "mynet0",
"vlan": 100,
"vlanTrunk": [
{ "id": 101 },
{ "minID": 200, "maxID": 299 }
],
"ipam": {}
}
```

## Example L2-only, disabled interface configuration
```json
{
Expand Down Expand Up @@ -94,9 +78,44 @@ If the bridge is missing, the plugin will create one on first use and, if gatewa
* `macspoofchk` (boolean, optional): Enables mac spoof check, limiting the traffic originating from the container to the mac address of the interface. Defaults to false.
* `disableContainerInterface` (boolean, optional): Set the container interface (veth peer inside the container netns) state down. When enabled, IPAM cannot be used.

*Note:* The VLAN parameter configures the VLAN tag on the host end of the veth and also enables the vlan_filtering feature on the bridge interface.
## VLAN capabilities

The `vlan` and `vlanTrunk` parameters are mutually exclusive.

The `vlan` parameter configures the VLAN tag on the host end of the veth and also enables the vlan_filtering feature on the bridge interface. The VLAN_DEFAULT_PVID of the bridge (when set), is added by default on the port as a VID untagged on egress. This is in most cases not desirable. Use the `preserveDefaultVlan` parameter to remove it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to try and explain this with VLAN_DEFAULT_PVID as a side-note, just because it is not assured that a reader will understand what it means.
E.g. it is not clear who sets it in the first place.
But, as commented in the next comment, I would suggest to avoid adding bridge details here, just explain the configuration abstracted in this plugin and its outcomes.

Also, could you please clarify what "default" preserveDefaultVlan refers to?
And what is its default value (when it does not appear in the config)?

Maybe it is worth trying to explain what are the side effects of having this set or not.


The `vlanTrunk` parameter allows to add a single VID or a range of VIDs (see example below). The native vlan of the trunk is the VLAN_DEFAULT_PVID of the bridge. This VID is added by default on the port with PVID and UNTAGGED options enabled. There are two known limitations due to this. First, all trunk ports on the bridge have the same native vlan. Second, the default PVID of the bridge is currently not configurable.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to try and explain things from the bridge perspective. It would help if the explanation is limited to the CNI plugin configuration which abstract the bridge side details.
Is this possible?


To configure uplink for L2 network you need to allow the vlan on the uplink interface by using the following command ``` bridge vlan add vid VLAN_ID dev DEV```.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure what this is suppose to do exactly.
This tries to explain that the plugin will not do this and users should do it themselves?

I am guessing this relates to defining a trunk port from the bridge to the node interface.
Perhaps a small diagram will help explain this better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was there before. We could review this in another pr



### Example L2-only vlan configuration
```json
{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "bridge",
"bridge": "mynet0",
"vlan": 100,
"preserveDefaultVlan": false,
"ipam": {}
}
```

### Example L2-only vlan trunk configuration
```json
{
"cniVersion": "0.3.1",
"name": "mynet",
"type": "bridge",
"bridge": "mynet0",
"vlanTrunk": [
{ "id": 101 },
{ "minID": 200, "maxID": 299 }],
"ipam": {}
}
```
Comment on lines +92 to +117
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: It is just 'good to have' comment, so you can ignore that

How about to use latest CNI spec, "cniVersion": "1.0.0"? If you change "1.0.0", then you need to use "plugins"...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be good, indeed. However, most (if not all) of the configuration examples use version "0.3.1". So, let's create a separate PR where we update the version in the examples of all plugins.


*Note:* To configure uplink for L2 network you need to allow the vlan on the uplink interface by using the following command ``` bridge vlan add vid VLAN_ID dev DEV```.

## Interface configuration arguments reference

Expand Down