expose mac address as part of networks-status in pod yaml #240
Conversation
|
should be merged after #239 |
Pull Request Test Coverage Report for Build 5045314882Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
SchSeba
force-pushed
the
expose_mac_address
branch
from
c04f663 to
37f5417
Compare
|
Hi @adrianchiris when you have time please take a look :) |
SchSeba
force-pushed
the
expose_mac_address
branch
from
37f5417 to
340d370
Compare
SchSeba
force-pushed
the
expose_mac_address
branch
from
340d370 to
fd3bad0
Compare
SchSeba
force-pushed
the
expose_mac_address
branch
from
fd3bad0 to
fc28be5
Compare
|
LGTM, thanks @SchSeba |
SchSeba
force-pushed
the
expose_mac_address
branch
4 times, most recently
from
e377e15 to
a78912a
Compare
pkg/config/config.go
Outdated
| // if the device is on kernel mode we report that one back | ||
| // if not we check the administrative mac address on the PF | ||
| // if it is set and is not zero, report it. | ||
| func GetMacAddressToReport(netConf *sriovtypes.NetConf) string { |
There was a problem hiding this comment.
i like the name in the comment better :) (GetMacAddressForResult)
that said, please align one to the other if u prefer GetMacAddressToReport
the commit allow to expose the mac address in the cni result object also for devices that are attached to user-space drivers like vfio mac address selection method: 1. if the vf is attached to the kernel use it 2. if the vf is attached to user-space driver check the vf admin mac 2.1 if the vf admin mac is not 0 publish it 2.2 if the vf admin mac is 0 don't publish the info Signed-off-by: Sebastian Sch <sebassch@gmail.com>
SchSeba
force-pushed
the
expose_mac_address
branch
from
a78912a to
33f3ed9
Compare
| // if not we check the administrative mac address on the PF | ||
| // if it is set and is not zero, report it. | ||
| func GetMacAddressForResult(netConf *sriovtypes.NetConf) string { | ||
| if netConf.MAC != "" { |
There was a problem hiding this comment.
do we need this statement ?
if MAC is specified then we set as admin mac and in non-dpdk we set as effective as well.
so the two statements below will cover it.
plus it will match the description of the method.
There was a problem hiding this comment.
This will not work for DPDK + static mac.
because we use netConf.OrigVfState.AdminMAC that contains the original adminMac not the static one we request in the MAC object
There was a problem hiding this comment.
I missed the fact that what we return below is the "Original" VF state so indeed when we override, the only place we keep that value is in netconf.MAC
sorry for the confusion.
this commit passes the mac address in the cni result.
this will also work in case the vf uses a user-space driver like vfio as we have the mac address for it from the PF
Signed-off-by: Sebastian Sch sebassch@gmail.com