This repository has been archived by the owner on Mar 20, 2024. It is now read-only.
multiple device request issue #81
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the issue is if a single pod requests different devices from different pools it results in multiple uds servers serving the same container and all attempt to mount their uds to the pod as /tmp/afxdp.sock. A similar issue exists for the AFXDP_DEVICES env var that's set in each container. This patch fixes the first issue by mounting the xsksocket at /tmp/afxdp_dp/<netdev>/afxdp.sock, a similar issue also existed for the bpf map pinning support. Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
maryamtahhan
force-pushed
the
feat_hotfix_multiple_uds
branch
from
6a31a01 to
d717535
Compare
maryamtahhan
commented
Nov 7, 2023
Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
|
an example of what's set for a pod that requests 2 devices from 2 different pools [access, core] is the following: |
maryamtahhan
commented
Nov 27, 2023
Signed-off-by: Maryam Tahhan <mtahhan@redhat.com>
|
Tested with DPDK testpmd output DP logs |
|
CNDP PR CloudNativeDataPlane/cndp#351 |
garyloug
reviewed
Nov 28, 2023
| @@ -73,14 +73,14 @@ build: builddp buildcni | |||
| docker: ## Build docker image | |||
| @echo "****** Docker Image ******" | |||
| @echo | |||
| docker build -t localhost:5000/afxdp-device-plugin -f images/amd64.dockerfile . | |||
| docker build -t afxdp-device-plugin -f images/amd64.dockerfile . | |||
There was a problem hiding this comment.
@patrickog11 - can you remind me why we had "localhost:5000" here?
Is it needed? Was it just a workaround on our side?
garyloug
approved these changes
Nov 28, 2023
ovsrobot
pushed a commit
to ovsrobot/dpdk
that referenced
this pull request
Dec 15, 2023
With the original 'use_cni' implementation, (using a hardcoded socket rather than a configurable one), if a DPDK pod is requesting multiple net devices and these devices are from different pools, then the container attempts to mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake correctly with the AF_XDP DP. This patch addresses this by making the socket parameter configurable using a new vdev param called 'uds_path' and removing the previous 'use_cni' param. This patch also fixes incorrect references to the AF_XDP DP as CNI and updates the documentation with a working example. This change has been tested with the AF_XDP DP PR 81[1], with both single and multiple interfaces. [1] intel/afxdp-plugins-for-kubernetes#81 v6: * Add link to PR 81 in commit message * Add release notes changes to this patchset v5: * Fix alignment for ETH_AF_XDP_USE_DP_UDS_PATH_ARG * Remove use_cni references in af_xdp.rst v4: * Rename af_xdp_cni.rst to af_xdp_dp.rst * Removed all incorrect references to CNI throughout af_xdp PMD file. * Fixed Typos in af_xdp_dp.rst v3: * Remove `use_cni` vdev argument as it's no longer needed. * Update incorrect CNI references for the AF_XDP DP in the documentation. * Update the documentation to run a simple example with the AF_XDP DP plugin in K8s. v2: * Rename sock_path to uds_path. * Update documentation to reflect when CAP_BPF is needed. * Fix testpmd arguments in the provided example for Pods. * Use AF_XDP API to update the xskmap entry. Signed-off-by: Maryam Tahhan <mtahhan@redhat.com> Reviewed-by: Ciara Loftus <ciara.loftus@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot
pushed a commit
to ovsrobot/dpdk
that referenced
this pull request
Dec 22, 2023
The original 'use_cni' implementation, was added to enable support for the AF_XDP PMD in a K8s env without any escalated privileges. However 'use_cni' used a hardcoded socket rather than a configurable one. If a DPDK pod is requesting multiple net devices and these devices are from different pools, then the AF_XDP PMD attempts to mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake correctly with the AF_XDP DP. This patch addresses this by making the socket parameter configurable using a new vdev param called 'uds_path' and removing the previous 'use_cni' param. This change has been tested with the AF_XDP DP PR 81[1], with both single and multiple interfaces. This patch also renames the af_xdp_cni.rst doc to af_xdp_dp.rst and changes incorrect references to the DP as CNI. Lastly, this patch adds this feature to the release notes. [1] intel/afxdp-plugins-for-kubernetes#81 Signed-off-by: Maryam Tahhan <mtahhan@redhat.com> Reviewed-by: Ciara Loftus <ciara.loftus@intel.com> Reviewed-by: Shibin Koikkara Reeny <shibin.koikkara.reeny@intel.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
maryamtahhan
added a commit
to maryamtahhan/dpdk
that referenced
this pull request
Jan 8, 2024
The original 'use_cni' implementation, was added to enable support for the AF_XDP PMD in a K8s env without any escalated privileges. However 'use_cni' used a hardcoded socket rather than a configurable one. If a DPDK pod is requesting multiple net devices and these devices are from different pools, then the AF_XDP PMD attempts to mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake correctly with the AF_XDP DP. This patch addresses this by making the socket parameter configurable using a new vdev param called 'uds_path' and removing the previous 'use_cni' param. This change has been tested with the AF_XDP DP PR 81[1], with both single and multiple interfaces. This patch also renames the af_xdp_cni.rst doc to af_xdp_dp.rst and changes incorrect references to the DP as CNI. Lastly, this patch adds this feature to the release notes. [1] intel/afxdp-plugins-for-kubernetes#81 Signed-off-by: Maryam Tahhan <mtahhan@redhat.com> Reviewed-by: Ciara Loftus <ciara.loftus@intel.com> Reviewed-by: Shibin Koikkara Reeny <shibin.koikkara.reeny@intel.com> --- v7: * Give a more descriptive commit msg headline. * Fixup typos in documentation. v6: * Add link to PR 81 in commit message * Add release notes changes to this patchset v5: * Fix alignment for ETH_AF_XDP_USE_DP_UDS_PATH_ARG * Remove use_cni references in af_xdp.rst v4: * Rename af_xdp_cni.rst to af_xdp_dp.rst * Removed all incorrect references to CNI throughout af_xdp PMD file. * Fixed Typos in af_xdp_dp.rst v3: * Remove `use_cni` vdev argument as it's no longer needed. * Update incorrect CNI references for the AF_XDP DP in the documentation. * Update the documentation to run a simple example with the AF_XDP DP plugin in K8s. v2: * Rename sock_path to uds_path. * Update documentation to reflect when CAP_BPF is needed. * Fix testpmd arguments in the provided example for Pods. * Use AF_XDP API to update the xskmap entry.
ovsrobot
pushed a commit
to ovsrobot/dpdk
that referenced
this pull request
Feb 29, 2024
The original 'use_cni' implementation, was added to enable support for the AF_XDP PMD in a K8s env without any escalated privileges. However 'use_cni' used a hardcoded socket rather than a configurable one. If a DPDK pod is requesting multiple net devices and these devices are from different pools, then the AF_XDP PMD attempts to mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake correctly with the AF_XDP DP. This patch addresses this by making the socket parameter configurable using a new vdev param called 'dp_path' alongside the original 'use_cni' param. If the 'dp_path' parameter is not set alongside the 'use_cni' parameter, then it's configured inside the AF_XDP PMD (transparently to the user). This change has been tested with the AF_XDP DP PR 81[1], with both single and multiple interfaces. [1] intel/afxdp-plugins-for-kubernetes#81 Fixes: 7fc6ae5 ("net/af_xdp: support CNI Integration") Cc: stable@dpdk.org Signed-off-by: Maryam Tahhan <mtahhan@redhat.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
ovsrobot
pushed a commit
to ovsrobot/dpdk
that referenced
this pull request
Feb 29, 2024
The original 'use_cni' implementation, was added to enable support for the AF_XDP PMD in a K8s env without any escalated privileges. However 'use_cni' used a hardcoded socket rather than a configurable one. If a DPDK pod is requesting multiple net devices and these devices are from different pools, then the AF_XDP PMD attempts to mount all the netdev UDSes in the pod as /tmp/afxdp.sock. Which means that at best only 1 netdev will handshake correctly with the AF_XDP DP. This patch addresses this by making the socket parameter configurable using a new vdev param called 'dp_path' alongside the original 'use_cni' param. If the 'dp_path' parameter is not set alongside the 'use_cni' parameter, then it's configured inside the AF_XDP PMD (transparently to the user). This change has been tested with the AF_XDP DP PR 81[1], with both single and multiple interfaces. [1] intel/afxdp-plugins-for-kubernetes#81 Fixes: 7fc6ae5 ("net/af_xdp: support CNI Integration") Cc: stable@dpdk.org Signed-off-by: Maryam Tahhan <mtahhan@redhat.com> Signed-off-by: 0-day Robot <robot@bytheb.org>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue is if a single pod requests different devices from different pools it results in multiple uds servers serving the same container and all attempt to mount their uds to the pod as /tmp/afxdp.sock.
A similar issue exists for the AFXDP_DEVICES env var that's set in each container.
This patch fixes the first issue by mounting the xsk socket at /tmp/afxdp_dp//afxdp.sock
This patch fixes the second issue by setting the env var in the container to AFXDP_DEVICES_<pool_name>