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

Jump to bottom

enable ip address allocation for dpdk interface #142

Merged
merged 1 commit into from
Nov 12, 2020
Merged

enable ip address allocation for dpdk interface #142

merged 1 commit into from
Nov 12, 2020

Conversation

zshi-redhat
Copy link
Collaborator

@zshi-redhat zshi-redhat commented Oct 19, 2020 •
edited
Loading

For an external gateway pod that routes pod traffic to
external world via secondary dpdk interface, it requires
an IP address be allocated to the dpdk interface and exposed
in the multus network-status annotation.

This commit enables allocation of ipam IP address to dpdk
interface, but not configuring to the dpdk interface.

@zshi-redhat zshi-redhat requested a review from ahalimx86 October 19, 2020 07:11
@ahalimx86
Copy link
Collaborator

How the allocated IP address will be retrieved and used by the application? Using downward API?

@zshi-redhat
Copy link
Collaborator Author

How the allocated IP address will be retrieved and used by the application? Using downward API?

Right. The ip address will be contained in the CNI result and published to pod network-status annotation by Multus. application can get it via mapping the annotation context into container as env var or file via downward API.

adrianchiris
adrianchiris reviewed Oct 26, 2020
View reviewed changes
Copy link
Contributor

@adrianchiris adrianchiris left a comment

Choose a reason for hiding this comment

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

Overall this look OK to me, small comment on the cmdDel.

cmd/sriov/main.go Outdated

return fmt.Errorf("failed to open netns %s: %q", netns, err)
netns, err := ns.GetNS(args.Netns)
Copy link
Contributor

Choose a reason for hiding this comment

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

to avoid opening and closing ns fd , as its not being used in dpdk mode, you could move L181-196 into the if block at L197

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

moshe010
moshe010 reviewed Oct 27, 2020
View reviewed changes
Copy link

@moshe010 moshe010 left a comment

Choose a reason for hiding this comment

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

So we are changing a bit the behaviour from skip IPAM to call it and not configure it. I think it worth updating the README on as it change that "dpdk driver then the IPAM configuration will be ignored". Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember) maybe it worth to list the supported IPAM or the one you tested it with. Also reflect it in the code and the others will still be ignored.

Also "DPDK userspace driver config" section I think we need to add a note that not all pmds requires uio/vfio driver. (e.g Mellanox bifurcated pmd). Or maybe it related to different PR

@adrianchiris
Copy link
Contributor

+1 on updating docs

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember)

As far as i saw in containernetworking/plugins, ipam plugins dont access the interface inside the pod.

@zshi-redhat
Copy link
Collaborator Author

zshi-redhat commented Oct 27, 2020 •
edited
Loading

So we are changing a bit the behaviour from skip IPAM to call it and not configure it. I think it worth updating the README on as it change that "dpdk driver then the IPAM configuration will be ignored".

Updated the doc.

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember) maybe it worth to list the supported IPAM or the one you tested it with. Also reflect it in the code and the others will still be ignored.

I only tested host-local, but I think it should work for all IPAM plugins including dhcp.
It looks for the interface when ConfigureIface (https://github.com/k8snetworkplumbingwg/sriov-cni/blob/master/cmd/sriov/main.go#L144) is called, but not when allocating IPs.

Also "DPDK userspace driver config" section I think we need to add a note that not all pmds requires uio/vfio driver. (e.g Mellanox bifurcated pmd). Or maybe it related to different PR

I think we could add a bifurcated driver section in a separate PR.

@zshi-redhat
Copy link
Collaborator Author

+1 on updating docs

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember)

As far as i saw in containernetworking/plugins, ipam plugins dont access the interface inside the pod.

Right, in current code path, ConfigureIface will not be called for vfio/uio interface, so it shall not access the interface inside pod.

@moshe010
Copy link

+1 on updating docs

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember)

As far as i saw in containernetworking/plugins, ipam plugins dont access the interface inside the pod.

Right, in current code path, ConfigureIface will not be called for vfio/uio interface, so it shall not access the interface inside pod.

in the dhcp you have daemon using pod interface as far as I remember https://github.com/containernetworking/plugins/blob/master/plugins/ipam/dhcp/daemon.go#L64.

@adrianchiris
Copy link
Contributor

in the dhcp you have daemon using pod interface as far as I remember

found it.

https://github.com/containernetworking/plugins/blob/440dcc331b692e64bcf0893a22ceda1826ee9f61/plugins/ipam/dhcp/lease.go#L81

@zshi-redhat
Copy link
Collaborator Author

+1 on updating docs

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember)

As far as i saw in containernetworking/plugins, ipam plugins dont access the interface inside the pod.

Right, in current code path, ConfigureIface will not be called for vfio/uio interface, so it shall not access the interface inside pod.

in the dhcp you have daemon using pod interface as far as I remember https://github.com/containernetworking/plugins/blob/master/plugins/ipam/dhcp/daemon.go#L64.

you're right, I will add the dhcp ipam as exception in the doc.

@zshi-redhat
Copy link
Collaborator Author

in the dhcp you have daemon using pod interface as far as I remember

found it.

https://github.com/containernetworking/plugins/blob/440dcc331b692e64bcf0893a22ceda1826ee9f61/plugins/ipam/dhcp/lease.go#L81

thanks for the link!

@zshi-redhat
Copy link
Collaborator Author

+1 on updating docs

Also will it work with all IPAM e.g dhcp? (it look for the interface in side the POD as far as I remember)

As far as i saw in containernetworking/plugins, ipam plugins dont access the interface inside the pod.

Right, in current code path, ConfigureIface will not be called for vfio/uio interface, so it shall not access the interface inside pod.

in the dhcp you have daemon using pod interface as far as I remember https://github.com/containernetworking/plugins/blob/master/plugins/ipam/dhcp/daemon.go#L64.

you're right, I will add the dhcp ipam as exception in the doc.

added a note in README

@zshi-redhat
Copy link
Collaborator Author

@ahalim-intel hi, any other concern about this change?

@ahalimx86
Copy link
Collaborator

@ahalim-intel hi, any other concern about this change?

I have no concerns so far. We will be doing some testing with this patch and let you know.

martinkennelly
martinkennelly approved these changes Nov 12, 2020
View reviewed changes
Copy link
Member

@martinkennelly martinkennelly left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM. We passed merge criteria now.

@martinkennelly martinkennelly merged commit ab10608 into k8snetworkplumbingwg:master Nov 12, 2020
This was referenced Jan 29, 2021
Merged
Closed
@silenceper
Copy link

@zshi-redhat I have a question. In dpdk mode (the driver of vfio-pci I use), the ip setting of the network card is not performed. When should I set it?

@zshi-redhat
Copy link
Collaborator Author

@zshi-redhat I have a question. In dpdk mode (the driver of vfio-pci I use), the ip setting of the network card is not performed. When should I set it?

The allocated IP address is not set automatically to the VF, since the VF device is in vfio-pci driver mode, we cannot directly configure IP to the VF. It is on the application who shall get the allocated IP address and use it to form the packet.

@silenceper
Copy link

Thank you, I've now understood

@silenceper silenceper mentioned this pull request Aug 19, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martinkennelly martinkennelly martinkennelly approved these changes

@adrianchiris adrianchiris adrianchiris approved these changes

@moshe010 moshe010 moshe010 approved these changes

@ahalimx86 ahalimx86 Awaiting requested review from ahalimx86

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@zshi-redhat @ahalimx86 @adrianchiris @moshe010 @silenceper @martinkennelly