* [PATCH v3 04/13] datagram: consolidate datagram copy to iter helpers
From: Sagi Grimberg @ 2018-11-22 1:56 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-1-sagi@grimberg.me>
From: Sagi Grimberg <sagi@lightbitslabs.com>
skb_copy_datagram_iter and skb_copy_and_csum_datagram are essentialy
the same but with a couple of differences: The first is the copy
operation used which either a simple copy or a csum_and_copy, and the
second are the behavior on the "short copy" path where simply copy
needs to return the number of bytes successfully copied while csum_and_copy
needs to fault immediately as the checksum is partial.
Introduce __skb_datagram_iter that additionally accepts:
1. copy operation function pointer
2. private data that goes with the copy operation
3. fault_short flag to indicate the action on short copy
Suggested-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
---
net/core/datagram.c | 136 ++++++++++++++------------------------------
1 file changed, 42 insertions(+), 94 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index abe642181b64..382543302ae5 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -408,27 +408,20 @@ int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
}
EXPORT_SYMBOL(skb_kill_datagram);
-/**
- * skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
- * @skb: buffer to copy
- * @offset: offset in the buffer to start copying from
- * @to: iovec iterator to copy to
- * @len: amount of data to copy from buffer to iovec
- */
-int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
- struct iov_iter *to, int len)
+int __skb_datagram_iter(const struct sk_buff *skb, int offset,
+ struct iov_iter *to, int len, bool fault_short,
+ size_t (*cb)(const void *, size_t, void *, struct iov_iter *),
+ void *data)
{
int start = skb_headlen(skb);
int i, copy = start - offset, start_off = offset, n;
struct sk_buff *frag_iter;
- trace_skb_copy_datagram_iovec(skb, len);
-
/* Copy header. */
if (copy > 0) {
if (copy > len)
copy = len;
- n = copy_to_iter(skb->data + offset, copy, to);
+ n = cb(skb->data + offset, copy, data, to);
offset += n;
if (n != copy)
goto short_copy;
@@ -450,8 +443,8 @@ int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
if (copy > len)
copy = len;
- n = copy_to_iter(vaddr + frag->page_offset +
- offset - start, copy, to);
+ n = cb(vaddr + frag->page_offset +
+ offset - start, copy, data, to);
kunmap(page);
offset += n;
if (n != copy)
@@ -471,8 +464,8 @@ int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
if ((copy = end - offset) > 0) {
if (copy > len)
copy = len;
- if (skb_copy_datagram_iter(frag_iter, offset - start,
- to, copy))
+ if (__skb_datagram_iter(frag_iter, offset - start,
+ to, copy, short_copy, cb, data))
goto fault;
if ((len -= copy) == 0)
return 0;
@@ -493,11 +486,32 @@ int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
return -EFAULT;
short_copy:
- if (iov_iter_count(to))
+ if (fault_short || iov_iter_count(to))
goto fault;
return 0;
}
+
+static size_t simple_copy_to_iter(const void *addr, size_t bytes,
+ void *data __always_unused, struct iov_iter *i)
+{
+ return copy_to_iter(addr, bytes, i);
+}
+
+/**
+ * skb_copy_datagram_iter - Copy a datagram to an iovec iterator.
+ * @skb: buffer to copy
+ * @offset: offset in the buffer to start copying from
+ * @to: iovec iterator to copy to
+ * @len: amount of data to copy from buffer to iovec
+ */
+int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
+ struct iov_iter *to, int len)
+{
+ trace_skb_copy_datagram_iovec(skb, len);
+ return __skb_datagram_iter(skb, offset, to, len, false,
+ simple_copy_to_iter, NULL);
+}
EXPORT_SYMBOL(skb_copy_datagram_iter);
/**
@@ -648,87 +662,21 @@ int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *from)
}
EXPORT_SYMBOL(zerocopy_sg_from_iter);
+/**
+ * skb_copy_and_csum_datagram_iter - Copy datagram to an iovec iterator
+ * and update a checksum.
+ * @skb: buffer to copy
+ * @offset: offset in the buffer to start copying from
+ * @to: iovec iterator to copy to
+ * @len: amount of data to copy from buffer to iovec
+ * @csump: checksum pointer
+ */
static int skb_copy_and_csum_datagram(const struct sk_buff *skb, int offset,
struct iov_iter *to, int len,
__wsum *csump)
{
- int start = skb_headlen(skb);
- int i, copy = start - offset, start_off = offset;
- struct sk_buff *frag_iter;
- int pos = 0;
- int n;
-
- /* Copy header. */
- if (copy > 0) {
- if (copy > len)
- copy = len;
- n = csum_and_copy_to_iter(skb->data + offset, copy, csump, to);
- offset += n;
- if (n != copy)
- goto fault;
- if ((len -= copy) == 0)
- return 0;
- pos = copy;
- }
-
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- int end;
- const skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
-
- WARN_ON(start > offset + len);
-
- end = start + skb_frag_size(frag);
- if ((copy = end - offset) > 0) {
- __wsum csum2 = 0;
- struct page *page = skb_frag_page(frag);
- u8 *vaddr = kmap(page);
-
- if (copy > len)
- copy = len;
- n = csum_and_copy_to_iter(vaddr + frag->page_offset +
- offset - start, copy,
- &csum2, to);
- kunmap(page);
- offset += n;
- if (n != copy)
- goto fault;
- *csump = csum_block_add(*csump, csum2, pos);
- if (!(len -= copy))
- return 0;
- pos += copy;
- }
- start = end;
- }
-
- skb_walk_frags(skb, frag_iter) {
- int end;
-
- WARN_ON(start > offset + len);
-
- end = start + frag_iter->len;
- if ((copy = end - offset) > 0) {
- __wsum csum2 = 0;
- if (copy > len)
- copy = len;
- if (skb_copy_and_csum_datagram(frag_iter,
- offset - start,
- to, copy,
- &csum2))
- goto fault;
- *csump = csum_block_add(*csump, csum2, pos);
- if ((len -= copy) == 0)
- return 0;
- offset += copy;
- pos += copy;
- }
- start = end;
- }
- if (!len)
- return 0;
-
-fault:
- iov_iter_revert(to, offset - start_off);
- return -EFAULT;
+ return __skb_datagram_iter(skb, offset, to, len, true,
+ csum_and_copy_to_iter, csump);
}
__sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len)
--
2.17.1
^ permalink raw reply related
* [PATCH v3 03/13] iov_iter: pass void csum pointer to csum_and_copy_to_iter
From: Sagi Grimberg @ 2018-11-22 1:56 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-1-sagi@grimberg.me>
From: Sagi Grimberg <sagi@lightbitslabs.com>
The single caller to csum_and_copy_to_iter is skb_copy_and_csum_datagram
and we are trying to unite its logic with skb_copy_datagram_iter by passing
a callback to the copy function that we want to apply. Thus, we need
to make the checksum pointer private to the function.
Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
---
include/linux/uio.h | 2 +-
lib/iov_iter.c | 3 ++-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 55ce99ddb912..41d1f8d3313d 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -266,7 +266,7 @@ static inline void iov_iter_reexpand(struct iov_iter *i, size_t count)
{
i->count = count;
}
-size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
+size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csump, struct iov_iter *i);
size_t csum_and_copy_from_iter(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum, struct iov_iter *i);
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 7ebccb5c1637..db93531ca3e3 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1432,10 +1432,11 @@ bool csum_and_copy_from_iter_full(void *addr, size_t bytes, __wsum *csum,
}
EXPORT_SYMBOL(csum_and_copy_from_iter_full);
-size_t csum_and_copy_to_iter(const void *addr, size_t bytes, __wsum *csum,
+size_t csum_and_copy_to_iter(const void *addr, size_t bytes, void *csump,
struct iov_iter *i)
{
const char *from = addr;
+ __wsum *csum = csump;
__wsum sum, next;
size_t off = 0;
sum = *csum;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 02/13] datagram: open-code copy_page_to_iter
From: Sagi Grimberg @ 2018-11-22 1:56 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-1-sagi@grimberg.me>
From: Sagi Grimberg <sagi@lightbitslabs.com>
This will be useful to consolidate skb_copy_and_hash_datagram_iter and
skb_copy_and_csum_datagram to a single code path.
Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
---
net/core/datagram.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/core/datagram.c b/net/core/datagram.c
index 57f3a6fcfc1e..abe642181b64 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -445,11 +445,14 @@ int skb_copy_datagram_iter(const struct sk_buff *skb, int offset,
end = start + skb_frag_size(frag);
if ((copy = end - offset) > 0) {
+ struct page *page = skb_frag_page(frag);
+ u8 *vaddr = kmap(page);
+
if (copy > len)
copy = len;
- n = copy_page_to_iter(skb_frag_page(frag),
- frag->page_offset + offset -
- start, copy, to);
+ n = copy_to_iter(vaddr + frag->page_offset +
+ offset - start, copy, to);
+ kunmap(page);
offset += n;
if (n != copy)
goto short_copy;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 01/13] ath6kl: add ath6kl_ prefix to crypto_type
From: Sagi Grimberg @ 2018-11-22 1:55 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
In-Reply-To: <20181122015615.15763-1-sagi@grimberg.me>
From: Sagi Grimberg <sagi@lightbitslabs.com>
Prevent a namespace conflict as in following patches as skbuff.h will
include the crypto API.
Cc: Kalle Valo <kvalo@codeaurora.org>
Signed-off-by: Sagi Grimberg <sagi@lightbitslabs.com>
---
drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +-
drivers/net/wireless/ath/ath6kl/common.h | 2 +-
drivers/net/wireless/ath/ath6kl/wmi.c | 6 +++---
drivers/net/wireless/ath/ath6kl/wmi.h | 6 +++---
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath6kl/cfg80211.c b/drivers/net/wireless/ath/ath6kl/cfg80211.c
index e121187f371f..fa049c4ae315 100644
--- a/drivers/net/wireless/ath/ath6kl/cfg80211.c
+++ b/drivers/net/wireless/ath/ath6kl/cfg80211.c
@@ -1322,7 +1322,7 @@ static int ath6kl_cfg80211_set_default_key(struct wiphy *wiphy,
struct ath6kl_vif *vif = netdev_priv(ndev);
struct ath6kl_key *key = NULL;
u8 key_usage;
- enum crypto_type key_type = NONE_CRYPT;
+ enum ath6kl_crypto_type key_type = NONE_CRYPT;
ath6kl_dbg(ATH6KL_DBG_WLAN_CFG, "%s: index %d\n", __func__, key_index);
diff --git a/drivers/net/wireless/ath/ath6kl/common.h b/drivers/net/wireless/ath/ath6kl/common.h
index 4f82e8632d37..d6e5234f67a1 100644
--- a/drivers/net/wireless/ath/ath6kl/common.h
+++ b/drivers/net/wireless/ath/ath6kl/common.h
@@ -67,7 +67,7 @@ struct ath6kl_llc_snap_hdr {
__be16 eth_type;
} __packed;
-enum crypto_type {
+enum ath6kl_crypto_type {
NONE_CRYPT = 0x01,
WEP_CRYPT = 0x02,
TKIP_CRYPT = 0x04,
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.c b/drivers/net/wireless/ath/ath6kl/wmi.c
index 777acc564ac9..9d7ac1ab2d02 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.c
+++ b/drivers/net/wireless/ath/ath6kl/wmi.c
@@ -1849,9 +1849,9 @@ int ath6kl_wmi_connect_cmd(struct wmi *wmi, u8 if_idx,
enum network_type nw_type,
enum dot11_auth_mode dot11_auth_mode,
enum auth_mode auth_mode,
- enum crypto_type pairwise_crypto,
+ enum ath6kl_crypto_type pairwise_crypto,
u8 pairwise_crypto_len,
- enum crypto_type group_crypto,
+ enum ath6kl_crypto_type group_crypto,
u8 group_crypto_len, int ssid_len, u8 *ssid,
u8 *bssid, u16 channel, u32 ctrl_flags,
u8 nw_subtype)
@@ -2301,7 +2301,7 @@ int ath6kl_wmi_disctimeout_cmd(struct wmi *wmi, u8 if_idx, u8 timeout)
}
int ath6kl_wmi_addkey_cmd(struct wmi *wmi, u8 if_idx, u8 key_index,
- enum crypto_type key_type,
+ enum ath6kl_crypto_type key_type,
u8 key_usage, u8 key_len,
u8 *key_rsc, unsigned int key_rsc_len,
u8 *key_material,
diff --git a/drivers/net/wireless/ath/ath6kl/wmi.h b/drivers/net/wireless/ath/ath6kl/wmi.h
index a60bb49fe920..784940ba4c90 100644
--- a/drivers/net/wireless/ath/ath6kl/wmi.h
+++ b/drivers/net/wireless/ath/ath6kl/wmi.h
@@ -2556,9 +2556,9 @@ int ath6kl_wmi_connect_cmd(struct wmi *wmi, u8 if_idx,
enum network_type nw_type,
enum dot11_auth_mode dot11_auth_mode,
enum auth_mode auth_mode,
- enum crypto_type pairwise_crypto,
+ enum ath6kl_crypto_type pairwise_crypto,
u8 pairwise_crypto_len,
- enum crypto_type group_crypto,
+ enum ath6kl_crypto_type group_crypto,
u8 group_crypto_len, int ssid_len, u8 *ssid,
u8 *bssid, u16 channel, u32 ctrl_flags,
u8 nw_subtype);
@@ -2610,7 +2610,7 @@ int ath6kl_wmi_config_debug_module_cmd(struct wmi *wmi, u32 valid, u32 config);
int ath6kl_wmi_get_stats_cmd(struct wmi *wmi, u8 if_idx);
int ath6kl_wmi_addkey_cmd(struct wmi *wmi, u8 if_idx, u8 key_index,
- enum crypto_type key_type,
+ enum ath6kl_crypto_type key_type,
u8 key_usage, u8 key_len,
u8 *key_rsc, unsigned int key_rsc_len,
u8 *key_material,
--
2.17.1
^ permalink raw reply related
* [PATCH v3 00/13] TCP transport binding for NVMe over Fabrics
From: Sagi Grimberg @ 2018-11-22 1:55 UTC (permalink / raw)
To: linux-nvme
Cc: linux-block, netdev, Christoph Hellwig, Keith Busch,
David S. Miller
Changes from v2:
- fixed stupid missing symbol export for skb_copy_and_hash_datagram_iter
- dropped patch that moved err_work and connect_work to nvme_ctrl
- fixed maxr2t icreq validation
- got rid of host and target send/recv context structures by moving
the members directly to their parent structure along with some struct
documentation
- removed bh disable when locking the queue lock
- moved definition in nvme-tcp.h to appropriate patch
- added patch to rework nvme-cli trtype handling for discovery log entries
a bit
- rebased on top of nvme-4.21 branch
- cleaned up some checkpatch warnings
- collected review tags
Changes from v1:
- unified skb_copy_datagram_iter and skb_copy_and_csum_datagram (and the
new skb_hash_and_copy_datagram_iter) to a single code path
- removed nvmet modparam budgets (made them a define set to their default
values)
- fixed nvme-tcp host chained r2t transfers reported off-list
- made .install_queue callout return nvme status code
- Added some review tags
- rebased on top of nvme-4.21 branch (nvme tree) + sqflow disable patches
This patch set implements the NVMe over Fabrics TCP host and the target
drivers. Now NVMe over Fabrics can run on every Ethernet port in the world.
The implementation conforms to NVMe over Fabrics 1.1 specification (which
will include already publicly available NVMe/TCP transport binding, TP 8000).
The host driver hooks into the NVMe host stack and implements the TCP
transport binding for NVMe over Fabrics. The NVMe over Fabrics TCP host
driver is responsible for establishing a NVMe/TCP connection, TCP event
and error handling and data-plane messaging and stream processing.
The target driver hooks into the NVMe target core stack and implements
the TCP transport binding. The NVMe over Fabrics target driver is
responsible for accepting and establishing NVMe/TCP connections, TCP
event and error handling, and data-plane messaging and stream processing.
The implementation of both the host and target are fairly simple and
straight-forward. Every NVMe queue is backed by a TCP socket that provides
us reliable, in-order delivery of fabrics capsules and/or data.
All NVMe queues are sharded over a private bound workqueue such that we
always have a single context handling the byte stream and we don't need
to worry about any locking/serialization. In addition, close attention
was paid to a completely non-blocking data plane to minimize context
switching and/or unforced scheduling.
I piggybacked nvme-cli patches to the set for completeness.
Also, @netdev mailing list is cc'd as this patch set contains generic
helpers for online digest calculation (patches 1-3).
The patchset structure:
- patches 1-6 are prep to add a helper for digest calculation online
with data placement
- patches 7-9 are preparatory patches for NVMe/TCP
- patches 10-13 implements NVMe/TCP
- patches 14-17 are nvme-cli additions for NVMe/TCP
Thanks to the members of the Fabrics Linux Driver team that helped
development, testing and benchmarking this work.
Gitweb code is available at:
git://git.infradead.org/nvme.git nvme-tcp
Sagi Grimberg (13):
ath6kl: add ath6kl_ prefix to crypto_type
datagram: open-code copy_page_to_iter
iov_iter: pass void csum pointer to csum_and_copy_to_iter
datagram: consolidate datagram copy to iter helpers
iov_iter: introduce hash_and_copy_to_iter helper
datagram: introduce skb_copy_and_hash_datagram_iter helper
nvmet: Add install_queue callout
nvme-fabrics: allow user passing header digest
nvme-fabrics: allow user passing data digest
nvme-tcp: Add protocol header
nvmet-tcp: add NVMe over TCP target driver
nvmet: allow configfs tcp trtype configuration
nvme-tcp: add NVMe over TCP host driver
drivers/net/wireless/ath/ath6kl/cfg80211.c | 2 +-
drivers/net/wireless/ath/ath6kl/common.h | 2 +-
drivers/net/wireless/ath/ath6kl/wmi.c | 6 +-
drivers/net/wireless/ath/ath6kl/wmi.h | 6 +-
drivers/nvme/host/Kconfig | 15 +
drivers/nvme/host/Makefile | 3 +
drivers/nvme/host/fabrics.c | 10 +
drivers/nvme/host/fabrics.h | 4 +
drivers/nvme/host/tcp.c | 2290 ++++++++++++++++++++
drivers/nvme/target/Kconfig | 10 +
drivers/nvme/target/Makefile | 2 +
drivers/nvme/target/configfs.c | 1 +
drivers/nvme/target/fabrics-cmd.c | 9 +
drivers/nvme/target/nvmet.h | 1 +
drivers/nvme/target/tcp.c | 1734 +++++++++++++++
include/linux/nvme-tcp.h | 189 ++
include/linux/nvme.h | 1 +
include/linux/skbuff.h | 3 +
include/linux/uio.h | 5 +-
lib/iov_iter.c | 19 +-
net/core/datagram.c | 159 +-
21 files changed, 4366 insertions(+), 105 deletions(-)
create mode 100644 drivers/nvme/host/tcp.c
create mode 100644 drivers/nvme/target/tcp.c
create mode 100644 include/linux/nvme-tcp.h
--
2.17.1
^ permalink raw reply
* Re: [RFC v3 3/3] vxlan: handle underlay VRF changes
From: David Ahern @ 2018-11-22 1:48 UTC (permalink / raw)
To: Alexis Bauvin, Roopa Prabhu; +Cc: netdev, akherbouche
In-Reply-To: <363CE0A1-D608-4E24-841A-992428372C19@scaleway.com>
On 11/21/18 5:54 PM, Alexis Bauvin wrote:
>>> There is one issue I can see with SO_REUSEPORT (if my understanding of it is
>>> correct). From what I understood, enabling this option will balance incoming
>>> connections (for TCP) / dgrams (for UDP) based on a 4-tuple hash (sip, dip,
>>> sport, dport) between sockets listening on the same port.
>>
>> AFAIK there is no balancing done. There is an order to which socket is
>> selected - and it includes the VRF device if relevant.
>
> Maybe balance was not the correct word, "route" may be more appropriate. Still you
> understood me, thanks for the details!
>
> Yet, the "if relevant" part is interesting. Does enabling the
> net.ipv4.udp_l3mdev_accept sysctl counts as making vrfs not releavant? In that case,
> both sockets are treated equally, right?
If udp_l3mdev_accept is disabled the default VRF is treated like a real
VRF as opposed to no VRF (Vyatta's changes), meaning the scope of the
socket is just the VRF (or just the default VRF).
If udp_l3mdev_accept is enabled it allows an unbound UDP socket to work
across all VRFs.
Gives user's options for how to deploy their s/w especially when it
comes to all of the existing software that is has no idea about VRFs.
^ permalink raw reply
* Re: [Patch net] net: invert the check of detecting hardware RX checksum fault
From: Paweł Staszewski @ 2018-11-22 1:25 UTC (permalink / raw)
To: Cong Wang, Herbert Xu
Cc: Linux Kernel Network Developers, Tom Herbert, Eric Dumazet
In-Reply-To: <CAM_iQpXWofVOg+u3fLYgFfU9-aTd7CadYfGF_q2hz16Af4_xGw@mail.gmail.com>
W dniu 16.11.2018 o 21:06, Cong Wang pisze:
> On Thu, Nov 15, 2018 at 8:50 PM Herbert Xu <herbert@gondor.apana.org.au> wrote:
>> On Thu, Nov 15, 2018 at 06:23:38PM -0800, Cong Wang wrote:
>>>> Normally if the hardware's partial checksum is valid then we just
>>>> trust it and send the packet along. However, if the partial
>>>> checksum is invalid we don't trust it and we will compute the
>>>> whole checksum manually which is what ends up in sum.
>>> Not sure if I understand partial checksum here, but it is the
>>> CHECKSUM_COMPLETE case which I am trying to fix, not
>>> CHECKSUM_PARTIAL.
>> What I meant by partial checksum is the checksum produced by the
>> hardware on RX. In the kernel we call that CHECKSUM_COMPLETE.
>> CHECKSUM_PARTIAL is the absence of the substantial part of the
>> checksum which is something we use in the kernel primarily for TX.
>>
>> Yes the names are confusing :)
> Yeah, understood. The hardware provides skb->csum in this case, but
> we keep adjusting it each time when we change skb->data.
>
>
>>> So, in other word, a checksum *match* is the intended to detect
>>> this HW RX checksum fault?
>> Correct. Or more likely it's probably a bug in either the driver
>> or if there are overlaying code such as VLAN then in that code.
>>
>> Basically if the RX checksum is buggy, it's much more likely to
>> cause a valid packet to be rejected than to cause an invalid packet
>> to be accepted, because we still verify that checksum against the
>> pseudoheader. So we only attempt to catch buggy hardware/drivers
>> by doing a second manual verification for the case where the packet
>> is flagged as invalid.
> Hmm, now I see how it works. Actually it uses the differences between
> these two check's as the difference between hardware checksum with
> skb_checksum().
>
> I will send a patch to add a comment there to avoid confusion.
>
>
>>> Sure, my case is nearly same with Pawel's, except I have no vlan:
>>> https://marc.info/?l=linux-netdev&m=154086647601721&w=2
>> Can you please provide your backtrace?
> I already did:
> https://marc.info/?l=linux-netdev&m=154092211305599&w=2
>
> Note, the offending commit has been backported to 4.14, which
> is why I saw this warning. I have no idea why it is backported
> from the beginning, it is just an optimization, doesn't fix any bug,
> IMHO.
>
> Also, it is much harder for me to reproduce it than Pawel who
> saw the warning every second. Sometimes I need 1 hour to trigger
> it, sometimes other people here needs 10+ hours to trigger it.
By the way - changed network controller for vlans where i was receiving
rx csum fail to 82599 with ixgbe driver and
with mellanox:
[91584.359273] vlan980: hw csum failure
[91584.359278] CPU: 54 PID: 0 Comm: swapper/54 Not tainted 4.20.0-rc1+ #2
[91584.359279] Call Trace:
[91584.359282] <IRQ>
[91584.359290] dump_stack+0x46/0x5b
[91584.359296] __skb_checksum_complete+0x9b/0xb0
[91584.359301] icmp_rcv+0x51/0x1f0
[91584.359305] ip_local_deliver_finish+0x49/0xd0
[91584.359307] ip_local_deliver+0xb7/0xe0
[91584.359309] ? ip_sublist_rcv_finish+0x50/0x50
[91584.359310] ip_rcv+0x96/0xc0
[91584.359313] __netif_receive_skb_one_core+0x4b/0x70
[91584.359315] netif_receive_skb_internal+0x2f/0xc0
[91584.359316] napi_gro_receive+0xb0/0xd0
[91584.359320] mlx5e_handle_rx_cqe+0x78/0xd0
[91584.359321] mlx5e_poll_rx_cq+0xc4/0x970
[91584.359323] mlx5e_napi_poll+0xab/0xcb0
[91584.359325] net_rx_action+0xd9/0x300
[91584.359328] __do_softirq+0xd3/0x2d9
[91584.359333] irq_exit+0x7a/0x80
[91584.359334] do_IRQ+0x72/0xc0
[91584.359336] common_interrupt+0xf/0xf
[91584.359337] </IRQ>
[91584.359340] RIP: 0010:mwait_idle+0x74/0x1b0
[91584.359342] Code: ae f0 31 d2 65 48 8b 04 25 80 4c 01 00 48 89 d1 0f
01 c8 48 8b 00 48 c1 e8 03 83 e0 01 0f 85 26 01 00 00 48 89 c1 fb 0f 01
c9 <65> 8b 2d 95 8e 6b 7e 0f 1f 44 00 00 65 48 8b 04 25 80 4c 01 00 f0
[91584.359343] RSP: 0018:ffffc900034f3ec0 EFLAGS: 00000246 ORIG_RAX:
ffffffffffffffde
[91584.359344] RAX: 0000000000000000 RBX: 0000000000000036 RCX:
0000000000000000
[91584.359345] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
0000000000000000
[91584.359346] RBP: 0000000000000036 R08: 0000000000000000 R09:
0000000000000000
[91584.359346] R10: 00000001008b49bb R11: 0000000000000c00 R12:
0000000000000000
[91584.359347] R13: 0000000000000000 R14: 0000000000000000 R15:
0000000000000000
[91584.359352] do_idle+0x19f/0x1c0
[91584.359354] ? do_idle+0x4/0x1c0
[91584.359355] cpu_startup_entry+0x14/0x20
[91584.359360] start_secondary+0x165/0x190
[91584.359364] secondary_startup_64+0xa4/0xb0
With intel no errors.
>
> Let me see if I can add vlan on my side to make it more reproducible,
> it seems hard as our switch doesn't use vlan either.
>
> We have warnings with conntrack involved too, I can provide it too
> if you are interested.
>
> I tend to revert it for -stable, at least that is what I plan to do
> on my side unless there is a fix coming soon.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH net-next 00/16] mlxsw: Add VxLAN learning support
From: David Miller @ 2018-11-22 1:10 UTC (permalink / raw)
To: idosch; +Cc: netdev, bridge, jiri, petrm, roopa, nikolay, stephen, ivecera,
mlxsw
In-Reply-To: <20181121080141.16676-1-idosch@mellanox.com>
From: Ido Schimmel <idosch@mellanox.com>
Date: Wed, 21 Nov 2018 08:02:32 +0000
> This patchset adds VxLAN learning support in the mlxsw driver.
Looks great, series applied, thanks Ido.
^ permalink raw reply
* [RFC v4 5/5] vxlan: handle underlay VRF changes
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181122010713.3995-1-abauvin@scaleway.com>
When underlay VRF changes, either because the lower device itself changed,
or its VRF changed, this patch releases the current socket of the VXLAN
device and recreates another one in the right VRF. This allows for
on-the-fly change of the underlay VRF of a VXLAN device.
Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
drivers/net/vxlan.c | 82 +++++++++++++++++++
.../selftests/net/test_vxlan_under_vrf.sh | 4 -
2 files changed, 82 insertions(+), 4 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 8ba0a57ff958..131ee80a38f9 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3720,6 +3720,33 @@ struct net_device *vxlan_dev_create(struct net *net, const char *name,
}
EXPORT_SYMBOL_GPL(vxlan_dev_create);
+static int vxlan_reopen(struct vxlan_net *vn, struct vxlan_dev *vxlan)
+{
+ int ret = 0;
+
+ if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip) &&
+ !vxlan_group_used(vn, vxlan))
+ ret = vxlan_igmp_leave(vxlan);
+ vxlan_sock_release(vxlan);
+
+ if (ret < 0)
+ return ret;
+
+ ret = vxlan_sock_add(vxlan);
+ if (ret < 0)
+ return ret;
+
+ if (vxlan_addr_multicast(&vxlan->default_dst.remote_ip)) {
+ ret = vxlan_igmp_join(vxlan);
+ if (ret == -EADDRINUSE)
+ ret = 0;
+ if (ret)
+ vxlan_sock_release(vxlan);
+ }
+
+ return ret;
+}
+
static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
struct net_device *dev)
{
@@ -3742,6 +3769,55 @@ static void vxlan_handle_lowerdev_unregister(struct vxlan_net *vn,
unregister_netdevice_many(&list_kill);
}
+static void vxlan_handle_change_upper(struct vxlan_net *vn,
+ struct net_device *dev)
+{
+ struct vxlan_dev *vxlan, *next;
+
+ list_for_each_entry_safe(vxlan, next, &vn->vxlan_list, next) {
+ struct net_device *lower;
+ int err;
+
+ lower = __dev_get_by_index(vxlan->net,
+ vxlan->cfg.remote_ifindex);
+ if (!netdev_is_upper_master(lower, dev))
+ continue;
+
+ err = vxlan_reopen(vn, vxlan);
+ if (err < 0)
+ netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
+ err);
+ }
+}
+
+static void vxlan_handle_change(struct vxlan_net *vn, struct net_device *dev)
+{
+ struct vxlan_dev *vxlan = netdev_priv(dev);
+ struct vxlan_sock *sock;
+ int l3mdev_index = 0;
+
+#if IS_ENABLED(CONFIG_IPV6)
+ bool metadata = vxlan->cfg.flags & VXLAN_F_COLLECT_METADATA;
+ bool ipv6 = vxlan->cfg.flags & VXLAN_F_IPV6 || metadata;
+
+ sock = ipv6 ? rcu_dereference(vxlan->vn6_sock)
+ : rcu_dereference(vxlan->vn4_sock);
+#else
+ sock = rcu_dereference(vxlan->vn4_sock);
+#endif
+
+ if (vxlan->cfg.remote_ifindex)
+ l3mdev_index = l3mdev_master_upper_ifindex_by_index(
+ vxlan->net, vxlan->cfg.remote_ifindex);
+ if (sock->sock->sk->sk_bound_dev_if != l3mdev_index) {
+ int ret = vxlan_reopen(vn, vxlan);
+
+ if (ret < 0)
+ netdev_err(vxlan->dev, "Failed to reopen socket: %d\n",
+ ret);
+ }
+}
+
static int vxlan_netdevice_event(struct notifier_block *unused,
unsigned long event, void *ptr)
{
@@ -3756,6 +3832,12 @@ static int vxlan_netdevice_event(struct notifier_block *unused,
} else if (event == NETDEV_UDP_TUNNEL_PUSH_INFO ||
event == NETDEV_UDP_TUNNEL_DROP_INFO) {
vxlan_offload_rx_ports(dev, event == NETDEV_UDP_TUNNEL_PUSH_INFO);
+ } else if (event == NETDEV_CHANGEUPPER) {
+ vxlan_handle_change_upper(vn, dev);
+ } else if (event == NETDEV_CHANGE) {
+ if (dev->rtnl_link_ops &&
+ !strcmp(dev->rtnl_link_ops->kind, vxlan_link_ops.kind))
+ vxlan_handle_change(vn, dev);
}
return NOTIFY_DONE;
diff --git a/tools/testing/selftests/net/test_vxlan_under_vrf.sh b/tools/testing/selftests/net/test_vxlan_under_vrf.sh
index 9ee906d5d333..fccbb00f4305 100755
--- a/tools/testing/selftests/net/test_vxlan_under_vrf.sh
+++ b/tools/testing/selftests/net/test_vxlan_under_vrf.sh
@@ -76,11 +76,7 @@ ip netns exec vm-1 ping -c 1 -W 1 10.0.0.2 &> /dev/null || (echo FAIL; false)
# Move the underlay to a non-default VRF
ip netns exec hv-1 ip link set veth0 vrf vrf-underlay
-ip netns exec hv-1 ip link set veth0 down
-ip netns exec hv-1 ip link set veth0 up
ip netns exec hv-2 ip link set veth0 vrf vrf-underlay
-ip netns exec hv-2 ip link set veth0 down
-ip netns exec hv-2 ip link set veth0 up
echo "Check VM connectivity through VXLAN (underlay in a VRF)"
ip netns exec vm-1 ping -c 1 -W 1 10.0.0.2 &> /dev/null || (echo FAIL; false)
--
^ permalink raw reply related
* [RFC v4 3/5] vxlan: add support for underlay in non-default VRF
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181122010713.3995-1-abauvin@scaleway.com>
Creating a VXLAN device with is underlay in the non-default VRF makes
egress route lookup fail or incorrect since it will resolve in the
default VRF, and ingress fail because the socket listens in the default
VRF.
This patch binds the underlying UDP tunnel socket to the l3mdev of the
lower device of the VXLAN device. This will listen in the proper VRF and
output traffic from said l3mdev, matching l3mdev routing rules and
looking up the correct routing table.
When the VXLAN device does not have a lower device, or the lower device
is in the default VRF, the socket will not be bound to any interface,
keeping the previous behaviour.
The underlay l3mdev is deduced from the VXLAN lower device
(IFLA_VXLAN_LINK).
+----------+ +---------+
| | | |
| vrf-blue | | vrf-red |
| | | |
+----+-----+ +----+----+
| |
| |
+----+-----+ +----+----+
| | | |
| br-blue | | br-red |
| | | |
+----+-----+ +---+-+---+
| | |
| +-----+ +-----+
| | |
+----+-----+ +------+----+ +----+----+
| | lower device | | | |
| eth0 | <- - - - - - - | vxlan-red | | tap-red | (... more taps)
| | | | | |
+----------+ +-----------+ +---------+
Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
drivers/net/vxlan.c | 32 +++++--
.../selftests/net/test_vxlan_under_vrf.sh | 90 +++++++++++++++++++
2 files changed, 114 insertions(+), 8 deletions(-)
create mode 100755 tools/testing/selftests/net/test_vxlan_under_vrf.sh
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 27bd586b94b0..8ba0a57ff958 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -212,7 +212,7 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
* and enabled unshareable flags.
*/
static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
- __be16 port, u32 flags)
+ __be16 port, u32 flags, int ifindex)
{
struct vxlan_sock *vs;
@@ -221,7 +221,8 @@ static struct vxlan_sock *vxlan_find_sock(struct net *net, sa_family_t family,
hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
if (inet_sk(vs->sock->sk)->inet_sport == port &&
vxlan_get_sk_family(vs) == family &&
- vs->flags == flags)
+ vs->flags == flags &&
+ vs->sock->sk->sk_bound_dev_if == ifindex)
return vs;
}
return NULL;
@@ -261,7 +262,7 @@ static struct vxlan_dev *vxlan_find_vni(struct net *net, int ifindex,
{
struct vxlan_sock *vs;
- vs = vxlan_find_sock(net, family, port, flags);
+ vs = vxlan_find_sock(net, family, port, flags, ifindex);
if (!vs)
return NULL;
@@ -2172,6 +2173,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct rtable *rt;
__be16 df = 0;
+ if (!ifindex)
+ ifindex = sock4->sock->sk->sk_bound_dev_if;
+
rt = vxlan_get_route(vxlan, dev, sock4, skb, ifindex, tos,
dst->sin.sin_addr.s_addr,
&local_ip.sin.sin_addr.s_addr,
@@ -2210,6 +2214,9 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
} else {
struct vxlan_sock *sock6 = rcu_dereference(vxlan->vn6_sock);
+ if (!ifindex)
+ ifindex = sock6->sock->sk->sk_bound_dev_if;
+
ndst = vxlan6_get_route(vxlan, dev, sock6, skb, ifindex, tos,
label, &dst->sin6.sin6_addr,
&local_ip.sin6.sin6_addr,
@@ -2813,7 +2820,7 @@ static const struct ethtool_ops vxlan_ethtool_ops = {
};
static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
- __be16 port, u32 flags)
+ __be16 port, u32 flags, int ifindex)
{
struct socket *sock;
struct udp_port_cfg udp_conf;
@@ -2831,6 +2838,7 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
}
udp_conf.local_udp_port = port;
+ udp_conf.bind_ifindex = ifindex;
/* Open UDP socket */
err = udp_sock_create(net, &udp_conf, &sock);
@@ -2842,7 +2850,8 @@ static struct socket *vxlan_create_sock(struct net *net, bool ipv6,
/* Create new listen socket if needed */
static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
- __be16 port, u32 flags)
+ __be16 port, u32 flags,
+ int ifindex)
{
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_sock *vs;
@@ -2857,7 +2866,7 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, bool ipv6,
for (h = 0; h < VNI_HASH_SIZE; ++h)
INIT_HLIST_HEAD(&vs->vni_list[h]);
- sock = vxlan_create_sock(net, ipv6, port, flags);
+ sock = vxlan_create_sock(net, ipv6, port, flags, ifindex);
if (IS_ERR(sock)) {
kfree(vs);
return ERR_CAST(sock);
@@ -2894,11 +2903,17 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
struct vxlan_net *vn = net_generic(vxlan->net, vxlan_net_id);
struct vxlan_sock *vs = NULL;
struct vxlan_dev_node *node;
+ int l3mdev_index = 0;
+
+ if (vxlan->cfg.remote_ifindex)
+ l3mdev_index = l3mdev_master_upper_ifindex_by_index(
+ vxlan->net, vxlan->cfg.remote_ifindex);
if (!vxlan->cfg.no_share) {
spin_lock(&vn->sock_lock);
vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
- vxlan->cfg.dst_port, vxlan->cfg.flags);
+ vxlan->cfg.dst_port, vxlan->cfg.flags,
+ l3mdev_index);
if (vs && !refcount_inc_not_zero(&vs->refcnt)) {
spin_unlock(&vn->sock_lock);
return -EBUSY;
@@ -2907,7 +2922,8 @@ static int __vxlan_sock_add(struct vxlan_dev *vxlan, bool ipv6)
}
if (!vs)
vs = vxlan_socket_create(vxlan->net, ipv6,
- vxlan->cfg.dst_port, vxlan->cfg.flags);
+ vxlan->cfg.dst_port, vxlan->cfg.flags,
+ l3mdev_index);
if (IS_ERR(vs))
return PTR_ERR(vs);
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/tools/testing/selftests/net/test_vxlan_under_vrf.sh b/tools/testing/selftests/net/test_vxlan_under_vrf.sh
new file mode 100755
index 000000000000..9ee906d5d333
--- /dev/null
+++ b/tools/testing/selftests/net/test_vxlan_under_vrf.sh
@@ -0,0 +1,90 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This test is for checking VXLAN underlay in a non-default VRF.
+
+set -e
+
+cleanup() {
+ ip link del veth-hv-1 || true
+ ip link del veth-tap || true
+
+ for ns in hv-1 hv-2 vm-1 vm-2; do
+ ip netns del $ns || true
+ done
+}
+
+# Clean start
+cleanup &> /dev/null
+
+[[ $1 == "clean" ]] && exit 0
+
+trap cleanup EXIT
+
+# Setup "Hypervisors" simulated with netns
+ip link add veth-hv-1 type veth peer name veth-hv-2
+setup-hv-networking() {
+ hv=$1
+
+ ip netns add hv-$hv
+ ip link set veth-hv-$hv netns hv-$hv
+ ip netns exec hv-$hv ip link set veth-hv-$hv name veth0
+
+ ip netns exec hv-$hv ip link add vrf-underlay type vrf table 1
+ ip netns exec hv-$hv ip link set vrf-underlay up
+ ip netns exec hv-$hv ip addr add 172.16.0.$hv/24 dev veth0
+ ip netns exec hv-$hv ip link set veth0 up
+
+ ip netns exec hv-$hv ip link add br0 type bridge
+ ip netns exec hv-$hv ip link set br0 up
+
+ ip netns exec hv-$hv ip link add vxlan0 type vxlan id 10 local 172.16.0.$hv dev veth0 dstport 4789
+ ip netns exec hv-$hv ip link set vxlan0 master br0
+ ip netns exec hv-$hv ip link set vxlan0 up
+}
+setup-hv-networking 1
+setup-hv-networking 2
+
+# Check connectivity between HVs by pinging hv-2 from hv-1
+echo Checking HV connectivity
+ip netns exec hv-1 ping -c 1 -W 1 172.16.0.2 &> /dev/null || (echo FAIL; false)
+
+# Setups a "VM" simulated by a netns an a veth pair
+setup-vm() {
+ id=$1
+
+ ip netns add vm-$id
+ ip link add veth-tap type veth peer name veth-hv
+
+ ip link set veth-tap netns hv-$id
+ ip netns exec hv-$id ip link set veth-tap master br0
+ ip netns exec hv-$id ip link set veth-tap up
+
+ ip link set veth-hv netns vm-$id
+ ip netns exec vm-$id ip addr add 10.0.0.$id/24 dev veth-hv
+ ip netns exec vm-$id ip link set veth-hv up
+}
+setup-vm 1
+setup-vm 2
+
+# Setup VTEP routes to make ARP work
+ip netns exec hv-1 bridge fdb add 00:00:00:00:00:00 dev vxlan0 dst 172.16.0.2 self permanent
+ip netns exec hv-2 bridge fdb add 00:00:00:00:00:00 dev vxlan0 dst 172.16.0.1 self permanent
+
+echo "Check VM connectivity through VXLAN (underlay in the default VRF)"
+ip netns exec vm-1 ping -c 1 -W 1 10.0.0.2 &> /dev/null || (echo FAIL; false)
+
+# Move the underlay to a non-default VRF
+ip netns exec hv-1 ip link set veth0 vrf vrf-underlay
+ip netns exec hv-1 ip link set veth0 down
+ip netns exec hv-1 ip link set veth0 up
+ip netns exec hv-2 ip link set veth0 vrf vrf-underlay
+ip netns exec hv-2 ip link set veth0 down
+ip netns exec hv-2 ip link set veth0 up
+
+echo "Check VM connectivity through VXLAN (underlay in a VRF)"
+ip netns exec vm-1 ping -c 1 -W 1 10.0.0.2 &> /dev/null || (echo FAIL; false)
+
+echo SUCCESS
+
+cleanup &> /dev/null
--
^ permalink raw reply related
* [RFC v4 4/5] netdev: add netdev_is_upper_master
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181122010713.3995-1-abauvin@scaleway.com>
In preparation of next patch, this function allows to check if a device
is a master, be it direct or indirect, of another one. It walks up the
master chain until it finds the device, or there is no more master.
This allows to check e.g. if br-blue is a master of eth0:
+----------+
| vrf-blue |
+----+-----+
|
+----+----+
| br-blue |
+----+----+
|
+---+---+
| bond0 |
+--+-+--+
| |
+--+ +--+
| |
+---+--+ +--+---+
| eth0 | | eth1 |
+------+ +------+
Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
include/linux/netdevice.h | 1 +
net/core/dev.c | 17 +++++++++++++++++
2 files changed, 18 insertions(+)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d837dad24b4c..102f79337d7c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4212,6 +4212,7 @@ void *netdev_lower_dev_get_private(struct net_device *dev,
struct net_device *lower_dev);
void netdev_lower_state_changed(struct net_device *lower_dev,
void *lower_state_info);
+bool netdev_is_upper_master(struct net_device *dev, struct net_device *master);
/* RSS keys are 40 or 52 bytes long */
#define NETDEV_RSS_KEY_LEN 52
diff --git a/net/core/dev.c b/net/core/dev.c
index 93243479085f..12459036d0da 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7225,6 +7225,23 @@ void netdev_lower_state_changed(struct net_device *lower_dev,
}
EXPORT_SYMBOL(netdev_lower_state_changed);
+/**
+ * netdev_is_upper_master - Test if a device is a master, direct or indirect,
+ * of another one.
+ * @dev: device to start looking from
+ * @master: device to test if master of dev
+ */
+bool netdev_is_upper_master(struct net_device *dev, struct net_device *master)
+{
+ if (!dev)
+ return false;
+
+ if (dev->ifindex == master->ifindex)
+ return true;
+ return netdev_is_upper_master(netdev_master_upper_dev_get(dev), master);
+}
+EXPORT_SYMBOL(netdev_is_upper_master);
+
static void dev_change_rx_flags(struct net_device *dev, int flags)
{
const struct net_device_ops *ops = dev->netdev_ops;
--
^ permalink raw reply related
* [RFC v4 2/5] l3mdev: add function to retreive upper master
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181122010713.3995-1-abauvin@scaleway.com>
Existing functions to retreive the l3mdev of a device did not walk the
master chain to find the upper master. This patch adds a function to
find the l3mdev, even indirect through e.g. a bridge:
+----------+
| |
| vrf-blue |
| |
+----+-----+
|
|
+----+-----+
| |
| br-blue |
| |
+----+-----+
|
|
+----+-----+
| |
| eth0 |
| |
+----------+
This will properly resolve the l3mdev of eth0 to vrf-blue.
Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
include/net/l3mdev.h | 22 ++++++++++++++++++++++
net/l3mdev/l3mdev.c | 18 ++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/net/l3mdev.h b/include/net/l3mdev.h
index 3832099289c5..78fa0ac4613c 100644
--- a/include/net/l3mdev.h
+++ b/include/net/l3mdev.h
@@ -101,6 +101,17 @@ struct net_device *l3mdev_master_dev_rcu(const struct net_device *_dev)
return master;
}
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex);
+static inline
+int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
+{
+ rcu_read_lock();
+ ifindex = l3mdev_master_upper_ifindex_by_index_rcu(net, ifindex);
+ rcu_read_unlock();
+
+ return ifindex;
+}
+
u32 l3mdev_fib_table_rcu(const struct net_device *dev);
u32 l3mdev_fib_table_by_index(struct net *net, int ifindex);
static inline u32 l3mdev_fib_table(const struct net_device *dev)
@@ -207,6 +218,17 @@ static inline int l3mdev_master_ifindex_by_index(struct net *net, int ifindex)
return 0;
}
+static inline
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
+{
+ return 0;
+}
+static inline
+int l3mdev_master_upper_ifindex_by_index(struct net *net, int ifindex)
+{
+ return 0;
+}
+
static inline
struct net_device *l3mdev_master_dev_rcu(const struct net_device *dev)
{
diff --git a/net/l3mdev/l3mdev.c b/net/l3mdev/l3mdev.c
index 8da86ceca33d..309dee76724e 100644
--- a/net/l3mdev/l3mdev.c
+++ b/net/l3mdev/l3mdev.c
@@ -46,6 +46,24 @@ int l3mdev_master_ifindex_rcu(const struct net_device *dev)
}
EXPORT_SYMBOL_GPL(l3mdev_master_ifindex_rcu);
+/**
+ * l3mdev_master_upper_ifindex_by_index - get index of upper l3 master
+ * device
+ * @net: network namespace for device index lookup
+ * @ifindex: targeted interface
+ */
+int l3mdev_master_upper_ifindex_by_index_rcu(struct net *net, int ifindex)
+{
+ struct net_device *dev;
+
+ dev = dev_get_by_index_rcu(net, ifindex);
+ while (dev && !netif_is_l3_master(dev))
+ dev = netdev_master_upper_dev_get(dev);
+
+ return dev ? dev->ifindex : 0;
+}
+EXPORT_SYMBOL_GPL(l3mdev_master_upper_ifindex_by_index_rcu);
+
/**
* l3mdev_fib_table - get FIB table id associated with an L3
* master interface
--
^ permalink raw reply related
* [RFC v4 1/5] udp_tunnel: add config option to bind to a device
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
In-Reply-To: <20181122010713.3995-1-abauvin@scaleway.com>
UDP tunnel sockets are always opened unbound to a specific device. This
patch allow the socket to be bound on a custom device, which
incidentally makes UDP tunnels VRF-aware if binding to an l3mdev.
Signed-off-by: Alexis Bauvin <abauvin@scaleway.com>
Reviewed-by: Amine Kherbouche <akherbouche@scaleway.com>
Tested-by: Amine Kherbouche <akherbouche@scaleway.com>
---
include/net/udp_tunnel.h | 1 +
net/ipv4/udp_tunnel.c | 10 ++++++++++
net/ipv6/ip6_udp_tunnel.c | 9 +++++++++
3 files changed, 20 insertions(+)
diff --git a/include/net/udp_tunnel.h b/include/net/udp_tunnel.h
index fe680ab6b15a..9f7970d010f9 100644
--- a/include/net/udp_tunnel.h
+++ b/include/net/udp_tunnel.h
@@ -30,6 +30,7 @@ struct udp_port_cfg {
__be16 local_udp_port;
__be16 peer_udp_port;
+ int bind_ifindex;
unsigned int use_udp_checksums:1,
use_udp6_tx_checksums:1,
use_udp6_rx_checksums:1,
diff --git a/net/ipv4/udp_tunnel.c b/net/ipv4/udp_tunnel.c
index 6539ff15e9a3..dc68e15a4f72 100644
--- a/net/ipv4/udp_tunnel.c
+++ b/net/ipv4/udp_tunnel.c
@@ -20,6 +20,16 @@ int udp_sock_create4(struct net *net, struct udp_port_cfg *cfg,
if (err < 0)
goto error;
+ if (cfg->bind_ifindex) {
+ struct net_device *dev;
+
+ dev = __dev_get_by_index(net, cfg->bind_ifindex);
+ err = kernel_setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE,
+ dev->name, strlen(dev->name) + 1);
+ if (err < 0)
+ goto error;
+ }
+
udp_addr.sin_family = AF_INET;
udp_addr.sin_addr = cfg->local_ip;
udp_addr.sin_port = cfg->local_udp_port;
diff --git a/net/ipv6/ip6_udp_tunnel.c b/net/ipv6/ip6_udp_tunnel.c
index b283f293ee4a..fc3811ef8787 100644
--- a/net/ipv6/ip6_udp_tunnel.c
+++ b/net/ipv6/ip6_udp_tunnel.c
@@ -31,6 +31,15 @@ int udp_sock_create6(struct net *net, struct udp_port_cfg *cfg,
if (err < 0)
goto error;
}
+ if (cfg->bind_ifindex) {
+ struct net_device *dev;
+
+ dev = __dev_get_by_index(net, cfg->bind_ifindex);
+ err = kernel_setsockopt(sock, SOL_SOCKET, SO_BINDTODEVICE,
+ dev->name, strlen(dev->name) + 1);
+ if (err < 0)
+ goto error;
+ }
udp6_addr.sin6_family = AF_INET6;
memcpy(&udp6_addr.sin6_addr, &cfg->local_ip6,
--
^ permalink raw reply related
* [RFC v4 0/5] Add VRF support for VXLAN underlay
From: Alexis Bauvin @ 2018-11-22 1:07 UTC (permalink / raw)
To: dsa, roopa; +Cc: netdev, abauvin, akherbouche
v3 -> v4:
- rename vxlan_is_in_l3mdev_chain to netdev_is_upper master
- move it to net/core/dev.c
- make it return bool instead of int
- check if remote_ifindex is zero before resolving the l3mdev
- add testing script
v2 -> v3:
- fix build when CONFIG_NET_IPV6 is off
- fix build "unused l3mdev_master_upper_ifindex_by_index" build error with some
configs
v1 -> v2:
- move vxlan_get_l3mdev from vxlan driver to l3mdev driver as
l3mdev_master_upper_ifindex_by_index
- vxlan: rename variables named l3mdev_ifindex to ifindex
v0 -> v1:
- fix typos
We are trying to isolate the VXLAN traffic from different VMs with VRF as shown
in the schemas below:
+-------------------------+ +----------------------------+
| +----------+ | | +------------+ |
| | | | | | | |
| | tap-red | | | | tap-blue | |
| | | | | | | |
| +----+-----+ | | +-----+------+ |
| | | | | |
| | | | | |
| +----+---+ | | +----+----+ |
| | | | | | | |
| | br-red | | | | br-blue | |
| | | | | | | |
| +----+---+ | | +----+----+ |
| | | | | |
| | | | | |
| | | | | |
| +----+--------+ | | +--------------+ |
| | | | | | | |
| | vxlan-red | | | | vxlan-blue | |
| | | | | | | |
| +------+------+ | | +-------+------+ |
| | | | | |
| | VRF | | | VRF |
| | red | | | blue |
+-------------------------+ +----------------------------+
| |
| |
+---------------------------------------------------------+
| | | |
| | | |
| | +--------------+ | |
| | | | | |
| +---------+ eth0.2030 +---------+ |
| | 10.0.0.1/24 | |
| +-----+--------+ VRF |
| | green|
+---------------------------------------------------------+
|
|
+----+---+
| |
| eth0 |
| |
+--------+
iproute2 commands to reproduce the setup:
ip link add green type vrf table 1
ip link set green up
ip link add eth0.2030 link eth0 type vlan id 2030
ip link set eth0.2030 master green
ip addr add 10.0.0.1/24 dev eth0.2030
ip link set eth0.2030 up
ip link add blue type vrf table 2
ip link set blue up
ip link add br-blue type bridge
ip link set br-blue master blue
ip link set br-blue up
ip link add vxlan-blue type vxlan id 2 local 10.0.0.1 dev eth0.2030 \
port 4789
ip link set vxlan-blue master br-blue
ip link set vxlan-blue up
ip link set tap-blue master br-blue
ip link set tap-blue up
ip link add red type vrf table 3
ip link set red up
ip link add br-red type bridge
ip link set br-red master red
ip link set br-red up
ip link add vxlan-red type vxlan id 3 local 10.0.0.1 dev eth0.2030 \
port 4789
ip link set vxlan-red master br-red
ip link set vxlan-red up
ip link set tap-red master br-red
ip link set tap-red up
We faced some issue in the datapath, here are the details:
* Egress traffic:
The vxlan packets are sent directly to the default VRF because it's where the
socket is bound, therefore the traffic has a default route via eth0. the
workarount is to force this traffic to VRF green with ip rules.
* Ingress traffic:
When receiving the traffic on eth0.2030 the vxlan socket is unreachable from
VRF green. The workaround is to enable *udp_l3mdev_accept* sysctl, but
this breaks isolation between overlay and underlay: packets sent from
blue or red by e.g. a guest VM will be accepted by the socket, allowing
injection of VXLAN packets from the overlay.
This patch serie fixes the issues describe above by allowing VXLAN socket to be
bound to a specific VRF device therefore looking up in the correct table.
Alexis Bauvin (5):
udp_tunnel: add config option to bind to a device
l3mdev: add function to retreive upper master
vxlan: add support for underlay in non-default VRF
netdev: add netdev_is_upper_master
vxlan: handle underlay VRF changes
drivers/net/vxlan.c | 114 ++++++++++++++++--
include/linux/netdevice.h | 1 +
include/net/l3mdev.h | 22 ++++
include/net/udp_tunnel.h | 1 +
net/core/dev.c | 17 +++
net/ipv4/udp_tunnel.c | 10 ++
net/ipv6/ip6_udp_tunnel.c | 9 ++
net/l3mdev/l3mdev.c | 18 +++
.../selftests/net/test_vxlan_under_vrf.sh | 86 +++++++++++++
9 files changed, 270 insertions(+), 8 deletions(-)
create mode 100755 tools/testing/selftests/net/test_vxlan_under_vrf.sh
--
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: implement head drops in backlog queue
From: Eric Dumazet @ 2018-11-22 1:01 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Eric Dumazet, David Miller, netdev, Jean-Louis Dupond,
Neal Cardwell
In-Reply-To: <CAK6E8=doFyZ8FhObWDFd7C4rHsHnphCP8-EqKqCSGJdmaMET7g@mail.gmail.com>
On Wed, Nov 21, 2018 at 4:55 PM Yuchung Cheng <ycheng@google.com> wrote:
>
> To clarify I do think this patch set is overall useful so I only
> wanted to discuss the specifics of the head drop.
>
> It occurs to me we check the limit differently (one w/ 64KB more), so
> we may overcommit and trim more often than necessary?
Keep in mind that the normal limit might be hit while backlog is empty.
So the loop might have nothing to remove.
We only have an extra 64KB credit for this very particular case, so
that we still can queue this packet
_if_ the additional credit was not already consumed.
We do not want to add back the terrible bug fixed by :
commit 8eae939f1400326b06d0c9afe53d2a484a326871
Author: Zhu Yi <yi.zhu@intel.com>
Date: Thu Mar 4 18:01:40 2010 +0000
net: add limit for socket backlog
We got system OOM while running some UDP netperf testing on the loopback
device. The case is multiple senders sent stream UDP packets to a single
receiver via loopback on local host. Of course, the receiver is not able
to handle all the packets in time. But we surprisingly found that these
packets were not discarded due to the receiver's sk->sk_rcvbuf limit.
Instead, they are kept queuing to sk->sk_backlog and finally ate up all
the memory. We believe this is a secure hole that a none privileged user
can crash the system.
The root cause for this problem is, when the receiver is doing
__release_sock() (i.e. after userspace recv, kernel udp_recvmsg ->
skb_free_datagram_locked -> release_sock), it moves skbs from backlog to
sk_receive_queue with the softirq enabled. In the above case, multiple
busy senders will almost make it an endless loop. The skbs in the
backlog end up eat all the system memory.
The issue is not only for UDP. Any protocols using socket backlog is
potentially affected. The patch adds limit for socket backlog so that
the backlog size cannot be expanded endlessly.
Reported-by: Alex Shi <alex.shi@intel.com>
Cc: David Miller <davem@davemloft.net>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru
Cc: "Pekka Savola (ipv6)" <pekkas@netcore.fi>
Cc: Patrick McHardy <kaber@trash.net>
Cc: Vlad Yasevich <vladislav.yasevich@hp.com>
Cc: Sridhar Samudrala <sri@us.ibm.com>
Cc: Jon Maloy <jon.maloy@ericsson.com>
Cc: Allan Stephens <allan.stephens@windriver.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Signed-off-by: Zhu Yi <yi.zhu@intel.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: implement head drops in backlog queue
From: Yuchung Cheng @ 2018-11-22 0:54 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David Miller, netdev, Jean-Louis Dupond,
Neal Cardwell
In-Reply-To: <CANn89i+9iuhUUwOhdEUs+xzgus6B1NSY534g-fBvf8Ns9Sa1pQ@mail.gmail.com>
On Wed, Nov 21, 2018 at 4:18 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Wed, Nov 21, 2018 at 3:52 PM Eric Dumazet <edumazet@google.com> wrote:
>> This is basically what the patch does, the while loop breaks when we have freed
>> just enough skbs.
>
> Also this is the patch we tested with Jean-Louis on his host, bring
> very nice results,
> even from an old stack sender (the one that had problems with the SACK
> compression we just fixed)
>
> Keep in mind we are dealing here with the exception, I would not spend
> too much time
> testing another variant if this one simply works.
To clarify I do think this patch set is overall useful so I only
wanted to discuss the specifics of the head drop.
It occurs to me we check the limit differently (one w/ 64KB more), so
we may overcommit and trim more often than necessary?
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: Toshiaki Makita @ 2018-11-22 0:53 UTC (permalink / raw)
To: David Ahern, netdev@vger.kernel.org
Cc: Paweł Staszewski, Jesper Dangaard Brouer, Saeed Mahameed,
Jason Wang, Michael S. Tsirkin, David Miller
In-Reply-To: <1d9a6548-4d1d-6624-e808-6ab0460a8655@gmail.com>
On 2018/11/22 6:06, David Ahern wrote:
> Paweł ran some more XDP tests yesterday and from it found a couple of
> issues. One is a panic in the mlx5 driver unloading the bpf program
> (mlx5e_xdp_xmit); he will send a send a separate email for that problem.
>
> The problem I wanted to discuss here is statistics for XDP context. The
> short of it is that we need consistency in the counters across NIC
> drivers and virtual devices. Right now stats are specific to a driver
> with no clear accounting for the packets and bytes handled in XDP.
>
> For example virtio has some stats as device private data extracted via
> ethtool:
> $ ethtool -S eth2 | grep xdp
> ...
> rx_queue_3_xdp_packets: 5291
> rx_queue_3_xdp_tx: 0
> rx_queue_3_xdp_redirects: 5163
> rx_queue_3_xdp_drops: 0
> ...
> tx_queue_3_xdp_tx: 5163
> tx_queue_3_xdp_tx_drops: 0
>
> And the standard counters appear to track bytes and packets for Rx, but
> not Tx if the packet is forwarded in XDP.
>
> Similarly, mlx5 has some counters (thanks to Jesper and Toke for helping
> out here):
>
> $ ethtool -S mlx5p1 | grep xdp
> rx_xdp_drop: 86468350180
> rx_xdp_redirect: 18860584
> rx_xdp_tx_xmit: 0
> rx_xdp_tx_full: 0
> rx_xdp_tx_err: 0
> rx_xdp_tx_cqe: 0
> tx_xdp_xmit: 0
> tx_xdp_full: 0
> tx_xdp_err: 0
> tx_xdp_cqes: 0
> ...
> rx3_xdp_drop: 86468350180
> rx3_xdp_redirect: 18860556
> rx3_xdp_tx_xmit: 0
> rx3_xdp_tx_full: 0
> rx3_xdp_tx_err: 0
> rx3_xdp_tx_cqes: 0
> ...
> tx0_xdp_xmit: 0
> tx0_xdp_full: 0
> tx0_xdp_err: 0
> tx0_xdp_cqes: 0
> ...
>
> And no accounting in standard stats for packets handled in XDP.
>
> And then if I understand Jesper's data correctly, the i40e driver does
> not have device specific data:
>
> $ ethtool -S i40e1 | grep xdp
> [NOTHING]
>
>
> But rather bumps the standard counters:
>
> sudo ./xdp_rxq_info --dev i40e1 --action XDP_DROP
>
> Running XDP on dev:i40e1 (ifindex:3) action:XDP_DROP options:no_touch
> XDP stats CPU pps issue-pps
> XDP-RX CPU 1 36,156,872 0
> XDP-RX CPU total 36,156,872
>
> RXQ stats RXQ:CPU pps issue-pps
> rx_queue_index 1:1 36,156,878 0
> rx_queue_index 1:sum 36,156,878
>
>
> $ ethtool_stats.pl --dev i40e1
>
> Show adapter(s) (i40e1) statistics (ONLY that changed!)
> Ethtool(i40e1 ) stat: 2711292859 ( 2,711,292,859) <= port.rx_bytes /sec
> Ethtool(i40e1 ) stat: 6274204 ( 6,274,204) <=
> port.rx_dropped /sec
> Ethtool(i40e1 ) stat: 42363867 ( 42,363,867) <=
> port.rx_size_64 /sec
> Ethtool(i40e1 ) stat: 42363950 ( 42,363,950) <=
> port.rx_unicast /sec
> Ethtool(i40e1 ) stat: 2165051990 ( 2,165,051,990) <= rx-1.bytes /sec
> Ethtool(i40e1 ) stat: 36084200 ( 36,084,200) <= rx-1.packets /sec
> Ethtool(i40e1 ) stat: 5385 ( 5,385) <= rx_dropped /sec
> Ethtool(i40e1 ) stat: 36089727 ( 36,089,727) <= rx_unicast /sec
>
>
> We really need consistency in the counters and at a minimum, users
> should be able to track packet and byte counters for both Rx and Tx
> including XDP.
>
> It seems to me the Rx and Tx packet, byte and dropped counters returned
> for the standard device stats (/proc/net/dev, ip -s li show, ...) should
> include all packets managed by the driver regardless of whether they are
> forwarded / dropped in XDP or go up the Linux stack. This also aligns
Agreed. When I introduced virtio_net XDP counters, I just forgot to
update tx packets/bytes counters on ndo_xdp_xmit. Probably I thought it
is handled by free_old_xmit_skbs.
Toshiaki Makita
> with mlxsw and the stats it shows which are packets handled by the hardware.
>
>>From there the private stats can include XDP specifics as desired --
> like the drops and redirects but that those should be add-ons and even
> here some consistency makes life easier for users.
>
> The same standards should be also be applied to virtual devices built on
> top of the ports -- e.g, vlans. I have an API now that allows bumping
> stats for vlan devices.
>
> Keeping the basic xdp packets in the standard counters allows Paweł, for
> example, to continue to monitor /proc/net/dev.
>
> Can we get agreement on this? And from there, get updates to the mlx5
> and virtio drivers?
>
> David
>
>
^ permalink raw reply
* Re: [RFC v3 3/3] vxlan: handle underlay VRF changes
From: Alexis Bauvin @ 2018-11-22 0:54 UTC (permalink / raw)
To: David Ahern, Roopa Prabhu; +Cc: netdev, akherbouche
In-Reply-To: <f738ce8a-6726-80c5-1961-3067b1f41566@cumulusnetworks.com>
Le 21 nov. 2018 à 20:28, David Ahern <dsa@cumulusnetworks.com> a écrit :
> On 11/21/18 7:05 AM, Alexis Bauvin wrote:
>> Le 20 nov. 2018 à 18:09, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>> On 11/20/18 9:58 AM, Alexis Bauvin wrote:
>>>> A socket bound to vrf-blue listens on *:4789, thus owning the port. If moving an
>>>> underlay to the default vrf (ip link set dummy-b nomaster), a new socket will be
>>>> created, unbound to any interface and listening on *:4789. However, because it
>>>> will be in the default vrf, it will try to take ownership of port 4789 on ALL
>>>> vrfs, and fail because this port is already owned in vrf-blue for vxlan-a.
>>>
>>> SO_REUSEPORT will fix that and incoming traffic through a vrf and
>>> default (non-)vrf will work. The recent changes by Vyatta provide even
>>> better isolation of default vrf and overlapping ports.
>>
>> Did not think about that one, I will give it a shot.
>>
>> There is one issue I can see with SO_REUSEPORT (if my understanding of it is
>> correct). From what I understood, enabling this option will balance incoming
>> connections (for TCP) / dgrams (for UDP) based on a 4-tuple hash (sip, dip,
>> sport, dport) between sockets listening on the same port.
>
> AFAIK there is no balancing done. There is an order to which socket is
> selected - and it includes the VRF device if relevant.
Maybe balance was not the correct word, "route" may be more appropriate. Still you
understood me, thanks for the details!
Yet, the "if relevant" part is interesting. Does enabling the
net.ipv4.udp_l3mdev_accept sysctl counts as making vrfs not releavant? In that case,
both sockets are treated equally, right?
>>
>> If we have two separate vxlan fabrics, with one underlay in the default vrf, and
>> another in some random vrf. Since the socket for the default vrf would own the
>> port on all vrfs, the port would effectively be reused between the two vrfs.
>> Wouldn't vxlan packets be directed to "random" (as in not related to the vxlan
>> fabric itself) sockets, meaning a complete mix of the two fabrics? This would
>> imply a complete drop of the packets not directed to the correct socket.
>>
>> I guess the Vyatta changes you are talking about are "vrf: allow simultaneous
>> service instances in default and other VRFs"? If so, it looks like it would
>> solve the default vrf problem, not even requiring SO_REUSEPORT.
>>
>
> yes.
So it would seem with the above that SO_REUSEPORT is a solution when not using
Vyatta's changes, which are a solution themselves (I will test those anyways).
^ permalink raw reply
* Re: [RFC v3 0/3] Add VRF support for VXLAN underlay
From: Alexis Bauvin @ 2018-11-22 0:47 UTC (permalink / raw)
To: David Ahern, roopa; +Cc: netdev, akherbouche
In-Reply-To: <d56867c5-9410-1256-eff0-48dfe0f25c5d@cumulusnetworks.com>
Le 21 nov. 2018 à 20:26, David Ahern <dsa@cumulusnetworks.com> a écrit :
>
> On 11/21/18 6:30 AM, Alexis Bauvin wrote:
>> Le 20 nov. 2018 à 22:45, David Ahern <dsa@cumulusnetworks.com> a écrit :
>>>
>>> On 11/20/18 7:23 AM, Alexis Bauvin wrote:
>>>> We are trying to isolate the VXLAN traffic from different VMs with VRF as shown
>>>> in the schemas below:
>>>>
>>>> +-------------------------+ +----------------------------+
>>>> | +----------+ | | +------------+ |
>>>> | | | | | | | |
>>>> | | tap-red | | | | tap-blue | |
>>>> | | | | | | | |
>>>> | +----+-----+ | | +-----+------+ |
>>>> | | | | | |
>>>> | | | | | |
>>>> | +----+---+ | | +----+----+ |
>>>> | | | | | | | |
>>>> | | br-red | | | | br-blue | |
>>>> | | | | | | | |
>>>> | +----+---+ | | +----+----+ |
>>>> | | | | | |
>>>> | | | | | |
>>>> | | | | | |
>>>> | +----+--------+ | | +--------------+ |
>>>> | | | | | | | |
>>>> | | vxlan-red | | | | vxlan-blue | |
>>>> | | | | | | | |
>>>> | +------+------+ | | +-------+------+ |
>>>> | | | | | |
>>>> | | VRF | | | VRF |
>>>> | | red | | | blue |
>>>> +-------------------------+ +----------------------------+
>>>
>>> Roopa and I were discussing this setup and are puzzled by the VRF
>>> association here. Does br-red and br-blue have an address? The commands
>>> below do not show it and from our perspective seems odd for this
>>> scenario. If it does not have an address, then there is no reason for
>>> the VRF labeling.
>>
>> Yes, br-red and br-blue have an address. Addresses (both MAC and IP)
>> are anycast addresses, to make an anycast gateway for the VMs behind
>> the taps. This serves for a classical evpn symmetric distributed
>> routing, both between vxlans and towards external networks. I am using
>> FRR as a control plane to exchange bgp-evpn information between
>> hypervisors.
>>
>> The schematic could have been more complete, including more bridges
>> and more taps in the vrf, with an added bridge+vxlan for the l3vni.
>> I did not include them as I wanted to focus on the underlay itself
>> and not on what vxlan was used for.
>>
>> Here is a more complete example:
>> +---------+
>> | |
>> | vrf1000 |
>> | |
>> +--+-+-+--+
>> | | |
>> +---------------------+ | +---------------+
>> | | |
>> +------+------+ +------+------+ +----+---+
>> | | | | | |
>> | br0 | | br1 | | br1000 |
>> | 10.0.0.1/24 | | 10.0.1.1/24 | | |
>> | | | | +----+---+
>> +----+-+-+----+ +-----+-+-----+ |
>> | | | | | |
>> +-------+ | +-------+ +--+ +---+ |
>> | | | | | |
>> +--+---+ +---+--+ +----+---+ +---+--+ +---+----+ +-----+-----+
>> | | | | | | | | | | | |
>> | tap0 | | tap1 | | vxlan0 | | tap2 | | vxlan1 | | vxlan1000 |
>> | | | | | | | | | | | |
>> +------+ +------+ +--------+ +------+ +--------+ +-----------+
>> . . .
>> . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
>> .
>> . +---------+ +--------------+
>> . | | | |
>> . | vrf1001 | | vrf-underlay |
>> . | | | |
>> . +---+-+---+ +-------+------+
>> . | | |
>> . +---------+ +----------+ |
>> . | | |
>> . +------+------+ +----+---+ |
>> . | | | | |
>> . | br2 | | br1001 | |
>> . | 10.0.2.1/24 | | | |
>> . | | +----+---+ |
>> . +----+-+-+----+ | |
>> . | | | | |
>> . +-----+ | +-------+ | |
>> . | | | | +-------+-------+
>> . +---+--+ +--+---+ +---+----+ +-----+-----+ | |
>> . | | | | | | | | | eth0.2030 |
>> . | tap3 | | tap4 | | vxlan2 | | vxlan1001 | | 172.16.0.2/24 |
>> . | | | | | | | | | |
>> . +------+ +------+ +--------+ +-----------+ +---------------+
>> . . . ^ .
>> . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .
>> (vxlan lower device is eth0.2030) v
>> +----------------+
>> | |
>> | eth0 |
>> | 192.168.1.2/24 |
>> | |
>> +----------------+
>>
>> In this example, the underlay is rather simple. A more complex case
>> could be with several uplinks in the underlay vrf, and a dummy/loopback
>> with a /32 ip, used as a vxlan lower device and to bind routing daemons
>> (e.g. bgpd).
>>
>> +--------------+
>> | |
>> | vrf-underlay |
>> | |
>> +-----+-+-+----+
>> | | |
>> +---------------+ | +---------------+
>> | | |
>> +-------+-------+ +-------+-------+ +-------+--------+
>> | | | | | |
>> | eth1 | | eth2 | | dummy0 |
>> | 172.16.0.2/31 | | 172.16.0.4/31 | | 192.168.0.1/32 |
>> | | | | | |
>> +---------------+ +---------------+ +----------------+
>> .
>> . . . . . . . . . . . . . . . . . . . . . . .
>> (vxlan lower device is dummy0)
>
> Thanks for sending a more complete example. I forwarded it to some FRR
> folks as well.
My pleasure.
>>
>> As for default vrf, it would contain for example management traffic. We
>> don’t want to mix the underlay routing tables to other traffics.
>
> Management VRF? :-)
Definitely possible, indeed. This would be the best solution, if only I *could*.
Reality is, people are still suspicious of networking guys. The least we touch
their servers, the better they are. For hypervisors we can, but the rest of
them are a mix and match, and I need to be as generic as possible.
Telling others to get off the default vrf for their own management softwares
(for example) if they want to integrate into what I’m developing is quite the
challenge. I need to be as plug-and-play as possible, and an underlay vrf is the
best option to keep the default to whatever it was used for.
(And as weird as it might be, it is easier for me to tell them to upgrade to a
mainline kernel than to rewrite their networking setup)
>>
>>> Also, it would be good to have a unit test this case. Can you create a
>>> shell script that creates the setup and runs a few tests verifying
>>> connectivity? You can use network namespaces and veth pairs in place of
>>> the VM with a tap device. From there the functionality is the same.
>>> Tests can be initial VRF association for the vxlan lower device,
>>> changing the VRF to another device, and then changing again back to
>>> default VRF - checking proper connectivity for each.
>>
>> Sure! I’ve just finished writing it, but I am not sure of the best way
>> so send it. I guess I will have to add it to a v4 of the patches, in
>> tools/testing/selftests/net?
>
> yes, that would be good. Thank you for writing it.
Will be sent in next patch series.
^ permalink raw reply
* Re: [PATCH][next] tools/bpf: fix spelling mistake "memeory" -> "memory"
From: Daniel Borkmann @ 2018-11-22 11:17 UTC (permalink / raw)
To: Colin King, Alexei Starovoitov, Shuah Khan, netdev,
linux-kselftest
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20181122101345.16271-1-colin.king@canonical.com>
On 11/22/2018 11:13 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The CHECK message contains a spelling mistake, fix it.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied to bpf-next, thanks!
^ permalink raw reply
* Re: consistency for statistics with XDP mode
From: Saeed Mahameed @ 2018-11-22 0:21 UTC (permalink / raw)
To: pstaszewski@itcare.pl, toke@toke.dk, netdev@vger.kernel.org,
dsahern@gmail.com
Cc: davem@davemloft.net, jasowang@redhat.com, brouer@redhat.com,
mst@redhat.com
In-Reply-To: <651ddca3-91e0-5ad6-6afe-46aaa4bd24c9@itcare.pl>
On Wed, 2018-11-21 at 22:29 +0100, Paweł Staszewski wrote:
> W dniu 21.11.2018 o 22:14, Toke Høiland-Jørgensen pisze:
> > David Ahern <dsahern@gmail.com> writes:
> >
> > > Paweł ran some more XDP tests yesterday and from it found a
> > > couple of
> > > issues. One is a panic in the mlx5 driver unloading the bpf
> > > program
> > > (mlx5e_xdp_xmit); he will send a send a separate email for that
> > > problem.
> > Same as this one, I guess?
> >
> > https://marc.info/?l=linux-netdev&m=153855905619717&w=2
>
> Yes same as this one.
>
> When there is no traffic (for example with xdp_fwd program loaded)
> or
> there is not much traffic like 1k frames per second for icmp - i can
> load/unload without crashing kernel
>
> But when i push tests with pktgen and use more than >50k pps for udp
> -
> then unbinding xdp_fwd program makes kernel to panic :)
>
Yea, this is not precisely mlx5 issue. this is one of the issues we
discussed at LPC, and i think we all agreed that the XDP redirect
infrastructure must allow different driver to sync when they are
changing configurations or disabling XPD tx for a moment, so the fix
must be in the XDP redirect infrastructure.
here is the issue description and a temp fix that i provided to Toke:
https://marc.info/?l=linux-netdev&m=154023109526642&w=2
patch:
https://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git/commit/?h=topic/xdp-redirect-fix&id=a3652d03cc35fd3ad62744986c8ccaca74c9f20c
>
>
> > > The problem I wanted to discuss here is statistics for XDP
> > > context. The
> > > short of it is that we need consistency in the counters across
> > > NIC
> > > drivers and virtual devices. Right now stats are specific to a
> > > driver
> > > with no clear accounting for the packets and bytes handled in
> > > XDP.
> > >
> > > For example virtio has some stats as device private data
> > > extracted via
> > > ethtool:
> > > $ ethtool -S eth2 | grep xdp
> > > ...
> > > rx_queue_3_xdp_packets: 5291
> > > rx_queue_3_xdp_tx: 0
> > > rx_queue_3_xdp_redirects: 5163
> > > rx_queue_3_xdp_drops: 0
> > > ...
> > > tx_queue_3_xdp_tx: 5163
> > > tx_queue_3_xdp_tx_drops: 0
> > >
> > > And the standard counters appear to track bytes and packets for
> > > Rx, but
> > > not Tx if the packet is forwarded in XDP.
> > >
> > > Similarly, mlx5 has some counters (thanks to Jesper and Toke for
> > > helping
> > > out here):
> > >
> > > $ ethtool -S mlx5p1 | grep xdp
> > > rx_xdp_drop: 86468350180
> > > rx_xdp_redirect: 18860584
> > > rx_xdp_tx_xmit: 0
> > > rx_xdp_tx_full: 0
> > > rx_xdp_tx_err: 0
> > > rx_xdp_tx_cqe: 0
> > > tx_xdp_xmit: 0
> > > tx_xdp_full: 0
> > > tx_xdp_err: 0
> > > tx_xdp_cqes: 0
> > > ...
> > > rx3_xdp_drop: 86468350180
> > > rx3_xdp_redirect: 18860556
> > > rx3_xdp_tx_xmit: 0
> > > rx3_xdp_tx_full: 0
> > > rx3_xdp_tx_err: 0
> > > rx3_xdp_tx_cqes: 0
> > > ...
> > > tx0_xdp_xmit: 0
> > > tx0_xdp_full: 0
> > > tx0_xdp_err: 0
> > > tx0_xdp_cqes: 0
> > > ...
> > >
> > > And no accounting in standard stats for packets handled in XDP.
> > >
> > > And then if I understand Jesper's data correctly, the i40e driver
> > > does
> > > not have device specific data:
> > >
> > > $ ethtool -S i40e1 | grep xdp
> > > [NOTHING]
> > >
> > >
> > > But rather bumps the standard counters:
> > >
> > > sudo ./xdp_rxq_info --dev i40e1 --action XDP_DROP
> > >
> > > Running XDP on dev:i40e1 (ifindex:3) action:XDP_DROP
> > > options:no_touch
> > > XDP stats CPU pps issue-pps
> > > XDP-RX CPU 1 36,156,872 0
> > > XDP-RX CPU total 36,156,872
> > >
> > > RXQ stats RXQ:CPU pps issue-pps
> > > rx_queue_index 1:1 36,156,878 0
> > > rx_queue_index 1:sum 36,156,878
> > >
> > >
> > > $ ethtool_stats.pl --dev i40e1
> > >
> > > Show adapter(s) (i40e1) statistics (ONLY that changed!)
> > > Ethtool(i40e1 ) stat: 2711292859 ( 2,711,292,859) <=
> > > port.rx_bytes /sec
> > > Ethtool(i40e1 ) stat: 6274204 ( 6,274,204) <=
> > > port.rx_dropped /sec
> > > Ethtool(i40e1 ) stat: 42363867 ( 42,363,867) <=
> > > port.rx_size_64 /sec
> > > Ethtool(i40e1 ) stat: 42363950 ( 42,363,950) <=
> > > port.rx_unicast /sec
> > > Ethtool(i40e1 ) stat: 2165051990 ( 2,165,051,990) <= rx-
> > > 1.bytes /sec
> > > Ethtool(i40e1 ) stat: 36084200 ( 36,084,200) <= rx-
> > > 1.packets /sec
> > > Ethtool(i40e1 ) stat: 5385 ( 5,385) <=
> > > rx_dropped /sec
> > > Ethtool(i40e1 ) stat: 36089727 ( 36,089,727) <=
> > > rx_unicast /sec
> > >
> > >
> > > We really need consistency in the counters and at a minimum,
> > > users
> > > should be able to track packet and byte counters for both Rx and
> > > Tx
> > > including XDP.
> > >
> > > It seems to me the Rx and Tx packet, byte and dropped counters
> > > returned
> > > for the standard device stats (/proc/net/dev, ip -s li show, ...)
> > > should
> > > include all packets managed by the driver regardless of whether
> > > they are
> > > forwarded / dropped in XDP or go up the Linux stack. This also
> > > aligns
> > > with mlxsw and the stats it shows which are packets handled by
> > > the hardware.
> > >
> > > From there the private stats can include XDP specifics as
> > > desired --
> > > like the drops and redirects but that those should be add-ons and
> > > even
> > > here some consistency makes life easier for users.
> > >
> > > The same standards should be also be applied to virtual devices
> > > built on
> > > top of the ports -- e.g, vlans. I have an API now that allows
> > > bumping
> > > stats for vlan devices.
> > >
> > > Keeping the basic xdp packets in the standard counters allows
> > > Paweł, for
> > > example, to continue to monitor /proc/net/dev.
> > >
> > > Can we get agreement on this? And from there, get updates to the
> > > mlx5
> > > and virtio drivers?
> > I'd say it sounds reasonable to include XDP in the normal traffic
> > counters, but having the detailed XDP-specific counters is quite
> > useful
> > as well... So can't we do both (for all drivers)?
> >
What are you thinking ?
reporting XDP_DROP in interface dropped counter ?
and XDP_TX/REDIRECT in the TX counter ?
XDP_ABORTED in the err/drop counter ?
how about having a special XDP command in the .ndo_bpf that would query
the standardized XDP stats ?
> > -Toke
> >
^ permalink raw reply
* Re: [PATCH net-next v2 1/4] enetc: Introduce basic PF and VF ENETC ethernet drivers
From: David Miller @ 2018-11-22 0:21 UTC (permalink / raw)
To: claudiu.manoil
Cc: netdev, linux-kernel, alexandru.marginean, catalin.horghidan
In-Reply-To: <1542737734-21756-2-git-send-email-claudiu.manoil@nxp.com>
From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: Tue, 20 Nov 2018 20:15:31 +0200
> diff --git a/drivers/net/ethernet/freescale/Makefile b/drivers/net/ethernet/freescale/Makefile
> index 3b4ff08..20e5c2f9 100644
> --- a/drivers/net/ethernet/freescale/Makefile
> +++ b/drivers/net/ethernet/freescale/Makefile
> @@ -23,3 +23,4 @@ obj-$(CONFIG_FSL_FMAN) += fman/
> obj-$(CONFIG_FSL_DPAA_ETH) += dpaa/
>
> obj-$(CONFIG_FSL_DPAA2_ETH) += dpaa2/
> +obj-$(CONFIG_NET_VENDOR_FREESCALE) += enetc/
The driver enable Kconfig option should guard traversing into the
driver subdirectory, not the vendor enable Kconfig knob.
^ permalink raw reply
* Re: net: thunderx: nicvf_xdp_setup error code path
From: David Miller @ 2018-11-22 0:18 UTC (permalink / raw)
To: lorenzo.bianconi; +Cc: netdev, sgoutham
In-Reply-To: <20181120175635.GA21691@localhost.localdomain>
From: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date: Tue, 20 Nov 2018 18:56:36 +0100
> Hi all,
>
> looking at thunderx XDP support I noticed that nic->xdp_prog pointer in
> nicvf_xdp_setup is not actually set to NULL if bpf_prog_add fails but it
> is initialized with bpf_prog_add error code. xdp_prog pointer value is used in
> the driver to verify if XDP is currently enabled.
> Moreover nicvf_xdp_setup does not report to the userspace any error code in
> case of failure.
> I wrote the following patch to fix the reported issues. Please note I just
> compiled it, not actually tested since I have no thunderx nic at the moment.
>
> @Sunil: could you please give it a whirl? If it is ok I will post a formal
> patch, thanks
Sunil did review this, so please resubmit formally.
^ permalink raw reply
* Re: [PATCH net-next 3/3] tcp: implement head drops in backlog queue
From: Eric Dumazet @ 2018-11-22 0:18 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Eric Dumazet, David Miller, netdev, Jean-Louis Dupond,
Neal Cardwell
In-Reply-To: <CANn89iKzg-Osj35=LtCMuA+A3hjEWojAThjvEhbX68+zxCL2_A@mail.gmail.com>
On Wed, Nov 21, 2018 at 3:52 PM Eric Dumazet <edumazet@google.com> wrote:
> This is basically what the patch does, the while loop breaks when we have freed
> just enough skbs.
Also this is the patch we tested with Jean-Louis on his host, bring
very nice results,
even from an old stack sender (the one that had problems with the SACK
compression we just fixed)
Keep in mind we are dealing here with the exception, I would not spend
too much time
testing another variant if this one simply works.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: lpc_eth: fix trivial comment typo
From: David Miller @ 2018-11-22 0:18 UTC (permalink / raw)
To: aclaudi; +Cc: netdev, vz
In-Reply-To: <b78920afc46c38a6a7d2cd843615bda06574c4b0.1542734728.git.aclaudi@redhat.com>
From: Andrea Claudi <aclaudi@redhat.com>
Date: Tue, 20 Nov 2018 18:30:30 +0100
> Fix comment typo rxfliterctrl -> rxfilterctrl
>
> Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox