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

MAC list broken with proper handling of set mac-vlan callback #261

Open
ScottDaniels opened this issue Feb 12, 2018 · 4 comments
Open

MAC list broken with proper handling of set mac-vlan callback #261

ScottDaniels opened this issue Feb 12, 2018 · 4 comments
Assignees

Comments

@ScottDaniels
Copy link
Contributor

To support the use of the 'set mac-vlan' callback which allows a guest to add MAC addresses to the white list in real-time, the capability of setting a static list of MAC addresses in the VFs config file may break.

Specifically, when a guest virtual machine initialises the vNIC a 'reset' is received (MAC address of all zeros) which causes VFd to clear the list of MAC addresses added to the VF. This behavour was added to emulate the behavour of various drivers and is correct. However, the feature of allowing MAC addresses to be supplied via the VF configuration is broken with this as the MAC addresses are cleared and as they were no t initially inserted by the guest, they will not be replaced.

To return VFd to support the ability to supply 'white list' MAC addresses in the VF configuration file, the following change is being proposed:

If the VF config contains one or more MAC addresses in the array, the first will be used as the default MAC address (visible to the guest) and the remainder will be added to the white list (guest will receive packets with these MAC addresses as the dest, and will be allowed to send packets with these MAC addresses as the source. Any set mac-vlan callbacks will be ignored by VFd.

When the VF config has an empty array for MAC addresses, or the field is missing from the config, VFd will cause a random MAC address to be defined as the default, and set mac-vlan callbacks will be processed allowing the guest to add/delete MAC addresses from the white list.

Please comment.

@pcarver
Copy link

pcarver commented Feb 12, 2018

Is there a strong reason to use the implicit logic of "first element in the array is special" vs having an explicit field to define the default MAC in addition to an array of equally non-special additional MACs?

@ScottDaniels
Copy link
Contributor Author

One less field to document, one less field for someone to forget to change. If you feel strongly about having a specific default field, then I'd suggest this change to the overall json (quotes omitted for simplicity):

macs: {
default: 04:30:86:11:04:17,
whilte_list [ m1, m2, ... mn ]
}

We'd have to support the old style for a while for backward compatibility.

@pcarver
Copy link

pcarver commented Feb 12, 2018

I would say that the "specialness" of the first element in the array ought to be documented, so doesn't save much documentation to document a separate field. The JSON you proposed looks exactly how I think it ought to look. I would suggest that a default or sample config file should include comment lines that indicate that if default MAC is missing it will be generated randomly (if you still want to support the random MAC generation you mentioned) and that if white_list is present that it will cause "set mac-vlan" callbacks to be ignored.

@ScottDaniels
Copy link
Contributor Author

Agree; the use of the first address as the default is already documented and is the current behaviour, as is the generation of the random address if the array is missing. Regardless doc changes are needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants