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

Sync 2023 06 27 #76

Merged
merged 138 commits into from Jul 4, 2023
Merged

Sync 2023 06 27 #76

merged 138 commits into from Jul 4, 2023

Conversation

SchSeba
Copy link
Contributor

@SchSeba SchSeba commented Jun 27, 2023

Fix merge between the u/s and d/s repo

zshi-redhat and others added 30 commits June 11, 2019 14:26
This change also include following  related files changes as well:

Makefile
images/entrypoint.sh
images/sriov-cni-daemonset.yaml

Change-Id: I4d5a8f15149fa166cd339ae8596ae0d7df71e2e5
This update will docker images in travis-ci.

Change-Id: I60d7d75708ba3bd54831cfccad5e8337c750f516
Also, restore the original MAC address on DEL action.

Note that both the effective and administrative MAC addresses are
independently set and restored (obviously depending on whether a netlink
driver is used).

This patch required pulling updated versions of a bunch of libraries,
starting from vishvananda/netlink and down the dependency stack.
* Add new `vlanQoS` field in the config spec.
* Call netlink library to set VLAN ID and VLAN QoS for a given VF interface.
* Reset VLAN ID and VLAN QoS configuration to default values on VF release.
* Add extra check to set VLAN QoS only if the VLAN ID is present.
* Add VLAN ID config validation.
* Update README.

Signed-off-by: Przemyslaw Lal <przemyslawx.lal@intel.com>
Use netlink library version that includes LinkSetVfVlanQos function.
Update golang.org/x/sys to the version comptible with the updated netlink library version.

Signed-off-by: Przemyslaw Lal <przemyslawx.lal@intel.com>
Change-Id: I0287a403355c5613dd39d265bf6e57307df7a014
This patch will set the following flags if set in the net-attach-def CRD:

spoofchk, trust, max_tx_rate

Also, these VF flags will be set to default values when the VF is
released:

spoofchk -> on
trust -> off
max_tx_rate -> 0, which disables rate limiting

Signed-off-by: vpickard <vpickard@redhat.com>
* added issue templates

* clarified that given file paths are most likely rather than default
When link_state is set, the link state of the corresponding VF will be
set to the passed value. Allowed values: auto, enable, disable. This is
useful in testing when e.g. in-PF switching should be enabled for
allocated VFs (which allows to connect pods running on the same host
without external switch or PF being plugged in the network).
CNI RuntimeConfig takes preference if configured,
envArgs can still be used for Mac configuration if
meta plugin(Multus) passes Mac as envArgs.

Fixes #104
Add support for configuring the min_tx_rate when
configured in the net-attach-def.

Note: min_tx_rate is only supported on Mellanox NICs.

For Intel NICs, do not configure min_tx_rate in the
net-attach-def, or, set the value to 0. Any other
config will cause the pod to fail to be created,
"Invalid Argument" in logs.

Signed-off-by: vpickard <vpickard@redhat.com>
This commit attempts to sort out (to some extent)
how a VF configuration is restored.

Until today, almost all VF configurations (except MAC)
where restored to some hardcoded value.
These values may not be desired by the system administrator
or may be invalid for some NIC driver implementation.

As an example, The behaviour of A VF where spoofchk is enabled
and admin mac address is zero is not well defined and is left
for the vendor driver implementors to define the behaviour.
This may cause the NIC's embedded switch to block traffic
from the VF, hence a POD will not be able to send traffic on that VF.

Today, when a user does not specify MAC and Spoofchk values in network
configuration will run into the issue described above when the VF is
reused for a POD.

To fix the issue, described above we take a similar approach to
how Administrative MAC is saved and restored today.

This commit:
  1. Defines a new VF state struct which will save VF configurations
     to be used during configuration restoration which shall be saved
     together with the network confguration in cache.
  2. Moves AdminMAC, EffectiveMAC attributes to the new struct
  3. Saves the VF's state during CmdAdd flow
  4. Conditionally restores all values to their saved state during
     ResetVFConfig()
  5. Modifies order when restoring VF confguration to first restore
     spoofchk and then Administrative MAC
  6. Enhances documentation under Mellanox to clarify the spoofchk
     and mac dependency.
Updated readme and split out advanced config guide.
With image imagePullPolicy set to IfNotPreset the user doesn't
need to pull the image manually
Use latest image version v2.3 in daemon templates
…ement.

Co-authored-by: zshi-redhat <zshi@redhat.com>
adrianchiris and others added 12 commits May 30, 2023 14:28
- Fix issue in pushing images to master/release
  with multi-arch build
- Bump action versions

Signed-off-by: adrianc <adrianc@nvidia.com>
Signed-off-by: adrianc <adrianc@nvidia.com>
Add both hardware and nic mac allocation retry
This allows users to set the allmulticast mode for a VF.

Signed-off-by: Marcelo Guerrero <marguerr@redhat.com>
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>
expose mac address as part of networks-status in pod yaml
Signed-off-by: Sebastian Sch <sebassch@gmail.com>
@SchSeba SchSeba changed the title Test Sync 2023 06 27 Jun 27, 2023
@@ -38,13 +38,13 @@ Creating VFs is outside the scope of the SR-IOV CNI plugin. [More information ab

To deploy SR-IOV CNI by itself on a Kubernetes 1.16+ cluster:

`kubectl apply -f images/k8s-v1.16/sriov-cni-daemonset.yaml`
`kubectl apply -f images/sriov-cni-daemonset.yaml`
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no images/sriov-cni-daemonset.yaml file downstream. We can keep the old path in the doc or bring images/sriov-cni-daemonset.yaml downstream. I think it's used for documentation only, right?

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 3, 2023

@SchSeba: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@bn222
Copy link
Contributor

bn222 commented Jul 4, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 4, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 4, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bn222, SchSeba

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 4, 2023
@openshift-merge-robot openshift-merge-robot merged commit 6a7fc55 into openshift:master Jul 4, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet