summaryrefslogtreecommitdiffstats
path: root/net/xdp/xsk.c
diff options
context:
space:
mode:
authorDaniel Borkmann <daniel@iogearbox.net>2018-10-05 09:31:01 +0200
committerDaniel Borkmann <daniel@iogearbox.net>2018-10-05 09:31:02 +0200
commitdf1ea77bc51ddb571530fd1b765b4c132095c419 (patch)
tree0b48b06adbcd305542c9e6d526bebdc522d8343a /net/xdp/xsk.c
parentbpf: typo fix in Documentation/networking/af_xdp.rst (diff)
parentxsk: simplify xdp_clear_umem_at_qid implementation (diff)
downloadlinux-df1ea77bc51ddb571530fd1b765b4c132095c419.tar.xz
linux-df1ea77bc51ddb571530fd1b765b4c132095c419.zip
Merge branch 'bpf-xsk-fix-mixed-mode'
Magnus Karlsson says: ==================== Previously, the xsk code did not record which umem was bound to a specific queue id. This was not required if all drivers were zero-copy enabled as this had to be recorded in the driver anyway. So if a user tried to bind two umems to the same queue, the driver would say no. But if copy-mode was first enabled and then zero-copy mode (or the reverse order), we mistakenly enabled both of them on the same umem leading to buggy behavior. The main culprit for this is that we did not store the association of umem to queue id in the copy case and only relied on the driver reporting this. As this relation was not stored in the driver for copy mode (it does not rely on the AF_XDP NDOs), this obviously could not work. This patch fixes the problem by always recording the umem to queue id relationship in the netdev_queue and netdev_rx_queue structs. This way we always know what kind of umem has been bound to a queue id and can act appropriately at bind time. To make the bind semantics consistent with ethtool queue manipulations and to facilitate the implementation of drivers, we also forbid decreasing the number of queues/channels with ethtool if there is an active AF_XDP socket in the set of queues that are disabled. Jakub, please take a look at your patches. The last one I had to change slightly to make it fit with the new interface xdp_get_umem_from_qid(). An added bonus with this function is that we, in the future, can also use it from the driver to get a umem, thus simplifying driver implementations (and later remove the umem from the NDO completely). Björn will mail patches, at a later point in time, using this in the i40e and ixgbe drivers, that removes a good chunk of code from the ZC implementations. I also made your code aware of Tx queues. If we create a socket that only has a Tx queue, then the queue id will refer to a Tx queue id only and could be larger than the available amount of Rx queues. Please take a look at it. Differences against v1: * Included patches from Jakub that forbids decreasing the number of active queues if a queue to be deactivated has an AF_XDP socket. These have been adapted somewhat to the new interfaces in patch 2. * Removed redundant check against real_num_[rt]x_queue in xsk_bind * Only need to test against real_num_[rt]x_queues in xdp_clear_umem_at_qid. Patch 1: Introduces a umem reference in the netdev_rx_queue and netdev_queue structs. Patch 2: Records which queue_id is bound to which umem and make sure that you cannot bind two different umems to the same queue_id. Patch 3: Pre patch to ethtool_set_channels. Patch 4: Forbid decreasing the number of active queues if a deactivated queue has an AF_XDP socket. Patch 5: Simplify xdp_clear_umem_at_qid now when ethtool cannot deactivate the queue id we are running on. ==================== Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Diffstat (limited to 'net/xdp/xsk.c')
-rw-r--r--net/xdp/xsk.c7
1 files changed, 0 insertions, 7 deletions
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 5a432dfee4ee..caeddad15b7c 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -419,13 +419,6 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
}
qid = sxdp->sxdp_queue_id;
-
- if ((xs->rx && qid >= dev->real_num_rx_queues) ||
- (xs->tx && qid >= dev->real_num_tx_queues)) {
- err = -EINVAL;
- goto out_unlock;
- }
-
flags = sxdp->sxdp_flags;
if (flags & XDP_SHARED_UMEM) {