* [PATCH net-next v2 10/10] net: qualcomm: rmnet: Add support for GSO
From: Subash Abhinov Kasiviswanathan @ 2018-01-03 21:43 UTC (permalink / raw)
To: davem, netdev, lkp; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515015787-6713-1-git-send-email-subashab@codeaurora.org>
Real devices may support scatter gather(SG), so enable SG on rmnet
devices to use GSO. GSO reduces CPU cycles by 20% for a rate of
146Mpbs for a single stream TCP connection.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index f7f57ce..570a227 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -190,6 +190,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
rmnet_dev->hw_features = NETIF_F_RXCSUM;
rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+ rmnet_dev->hw_features |= NETIF_F_SG;
rc = register_netdevice(rmnet_dev);
if (!rc) {
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 09/10] net: qualcomm: rmnet: Add support for TX checksum offload
From: Subash Abhinov Kasiviswanathan @ 2018-01-03 21:43 UTC (permalink / raw)
To: davem, netdev, lkp; +Cc: Subash Abhinov Kasiviswanathan
In-Reply-To: <1515015787-6713-1-git-send-email-subashab@codeaurora.org>
TX checksum offload applies to TCP / UDP packets which are not
fragmented using the MAPv4 checksum trailer. The following needs to be
done to have checksum computed in hardware -
1. Set the checksum start offset and inset offset.
2. Set the csum_enabled bit
3. Compute and set 1's complement of partial checksum field in
transport header.
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
.../net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 8 ++
drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h | 2 +
.../net/ethernet/qualcomm/rmnet/rmnet_map_data.c | 120 +++++++++++++++++++++
drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c | 1 +
4 files changed, 131 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
index 3409458..601edec 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
@@ -141,11 +141,19 @@ static int rmnet_map_egress_handler(struct sk_buff *skb,
additional_header_len = 0;
required_headroom = sizeof(struct rmnet_map_header);
+ if (port->data_format & RMNET_EGRESS_FORMAT_MAP_CKSUMV4) {
+ additional_header_len = sizeof(struct rmnet_map_ul_csum_header);
+ required_headroom += additional_header_len;
+ }
+
if (skb_headroom(skb) < required_headroom) {
if (pskb_expand_head(skb, required_headroom, 0, GFP_KERNEL))
goto fail;
}
+ if (port->data_format & RMNET_EGRESS_FORMAT_MAP_CKSUMV4)
+ rmnet_map_checksum_uplink_packet(skb, orig_dev);
+
map_header = rmnet_map_add_map_header(skb, additional_header_len, 0);
if (!map_header)
goto fail;
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
index ca9f473..6ce31e2 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map.h
@@ -89,5 +89,7 @@ struct rmnet_map_header *rmnet_map_add_map_header(struct sk_buff *skb,
int hdrlen, int pad);
void rmnet_map_command(struct sk_buff *skb, struct rmnet_port *port);
int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len);
+void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
+ struct net_device *orig_dev);
#endif /* _RMNET_MAP_H_ */
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 881c1dc..c74a6c5 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -171,6 +171,86 @@ static __sum16 *rmnet_map_get_csum_field(unsigned char protocol,
}
#endif
+static void rmnet_map_complement_ipv4_txporthdr_csum_field(void *iphdr)
+{
+ struct iphdr *ip4h = (struct iphdr *)iphdr;
+ void *txphdr;
+ u16 *csum;
+
+ txphdr = iphdr + ip4h->ihl * 4;
+
+ if (ip4h->protocol == IPPROTO_TCP || ip4h->protocol == IPPROTO_UDP) {
+ csum = (u16 *)rmnet_map_get_csum_field(ip4h->protocol, txphdr);
+ *csum = ~(*csum);
+ }
+}
+
+static void
+rmnet_map_ipv4_ul_csum_header(void *iphdr,
+ struct rmnet_map_ul_csum_header *ul_header,
+ struct sk_buff *skb)
+{
+ struct iphdr *ip4h = (struct iphdr *)iphdr;
+ __be16 *hdr = (__be16 *)ul_header, offset;
+
+ offset = htons((__force u16)(skb_transport_header(skb) -
+ (unsigned char *)iphdr));
+ ul_header->csum_start_offset = offset;
+ ul_header->csum_insert_offset = skb->csum_offset;
+ ul_header->csum_enabled = 1;
+ if (ip4h->protocol == IPPROTO_UDP)
+ ul_header->udp_ip4_ind = 1;
+ else
+ ul_header->udp_ip4_ind = 0;
+
+ /* Changing remaining fields to network order */
+ hdr++;
+ *hdr = htons((__force u16)*hdr);
+
+ skb->ip_summed = CHECKSUM_NONE;
+
+ rmnet_map_complement_ipv4_txporthdr_csum_field(iphdr);
+}
+
+#if IS_ENABLED(CONFIG_IPV6)
+static void rmnet_map_complement_ipv6_txporthdr_csum_field(void *ip6hdr)
+{
+ struct ipv6hdr *ip6h = (struct ipv6hdr *)ip6hdr;
+ void *txphdr;
+ u16 *csum;
+
+ txphdr = ip6hdr + sizeof(struct ipv6hdr);
+
+ if (ip6h->nexthdr == IPPROTO_TCP || ip6h->nexthdr == IPPROTO_UDP) {
+ csum = (u16 *)rmnet_map_get_csum_field(ip6h->nexthdr, txphdr);
+ *csum = ~(*csum);
+ }
+}
+
+static void
+rmnet_map_ipv6_ul_csum_header(void *ip6hdr,
+ struct rmnet_map_ul_csum_header *ul_header,
+ struct sk_buff *skb)
+{
+ __be16 *hdr = (__be16 *)ul_header, offset;
+
+ offset = htons((__force u16)(skb_transport_header(skb) -
+ (unsigned char *)ip6hdr));
+ ul_header->csum_start_offset = offset;
+ ul_header->csum_insert_offset = skb->csum_offset;
+ ul_header->csum_enabled = 1;
+ ul_header->udp_ip4_ind = 0;
+
+ /* Changing remaining fields to network order */
+ hdr++;
+ *hdr = htons((__force u16)*hdr);
+
+ skb->ip_summed = CHECKSUM_NONE;
+
+ rmnet_map_complement_ipv6_txporthdr_csum_field(ip6hdr);
+}
+#endif
+
/* Adds MAP header to front of skb->data
* Padding is calculated and set appropriately in MAP header. Mux ID is
* initialized to 0.
@@ -281,3 +361,43 @@ int rmnet_map_checksum_downlink_packet(struct sk_buff *skb, u16 len)
return 0;
}
+
+/* Generates UL checksum meta info header for IPv4 and IPv6 over TCP and UDP
+ * packets that are supported for UL checksum offload.
+ */
+void rmnet_map_checksum_uplink_packet(struct sk_buff *skb,
+ struct net_device *orig_dev)
+{
+ struct rmnet_map_ul_csum_header *ul_header;
+ void *iphdr;
+
+ ul_header = (struct rmnet_map_ul_csum_header *)
+ skb_push(skb, sizeof(struct rmnet_map_ul_csum_header));
+
+ if (unlikely(!(orig_dev->features &
+ (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM))))
+ goto sw_csum;
+
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ iphdr = (char *)ul_header +
+ sizeof(struct rmnet_map_ul_csum_header);
+
+ if (skb->protocol == htons(ETH_P_IP)) {
+ rmnet_map_ipv4_ul_csum_header(iphdr, ul_header, skb);
+ return;
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+#if IS_ENABLED(CONFIG_IPV6)
+ rmnet_map_ipv6_ul_csum_header(iphdr, ul_header, skb);
+ return;
+#else
+ goto sw_csum;
+#endif
+ }
+ }
+
+sw_csum:
+ ul_header->csum_start_offset = 0;
+ ul_header->csum_insert_offset = 0;
+ ul_header->csum_enabled = 0;
+ ul_header->udp_ip4_ind = 0;
+}
diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
index 879a2e0..f7f57ce 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_vnd.c
@@ -189,6 +189,7 @@ int rmnet_vnd_newlink(u8 id, struct net_device *rmnet_dev,
return -EBUSY;
rmnet_dev->hw_features = NETIF_F_RXCSUM;
+ rmnet_dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
rc = register_netdevice(rmnet_dev);
if (!rc) {
--
1.9.1
^ permalink raw reply related
* Re: bonding: Delete an error message for a failed memory allocation in bond_update_slave_arr()
From: Eric Dumazet @ 2018-01-03 21:58 UTC (permalink / raw)
To: Mahesh Bandewar (महेश बंडेवार),
SF Markus Elfring
Cc: linux-netdev, Andy Gospodarek, Jay Vosburgh, Veaceslav Falico,
LKML, kernel-janitors
In-Reply-To: <CAF2d9jh1EHUx3BzWghBWRAZt4acO6W-_eUXtD7dk2YBJZfuwsg@mail.gmail.com>
On Wed, 2018-01-03 at 11:28 -0800, Mahesh Bandewar (महेश बंडेवार)
wrote:
> On Wed, Jan 3, 2018 at 12:45 AM, SF Markus Elfring
> <elfring@users.sourceforge.net> wrote:
> > > > Omit an extra message for a memory allocation failure in this function.
> > > >
> > > > This issue was detected by using the Coccinelle software.
> > > >
> > >
> > > What is the issue with this message?
> >
> > * Is it redundant?
> >
> > * Would a Linux allocation failure report be already sufficient here?
> >
>
> If you see 8 out of 9 call sites in this file ignore the return value.
> This message in the log could give a clue when debugging. Unless it's
> spamming it's not harmful, or is it?
A failed kzalloc() would already give a complete stack trace.
Really the pr_err() adds no value here.
^ permalink raw reply
* Re: [PATCH net-next v2 05/10] net: qualcomm: rmnet: Set pacing rate
From: Eric Dumazet @ 2018-01-03 22:01 UTC (permalink / raw)
To: Subash Abhinov Kasiviswanathan, davem, netdev, lkp
In-Reply-To: <1515015787-6713-6-git-send-email-subashab@codeaurora.org>
On Wed, 2018-01-03 at 14:43 -0700, Subash Abhinov Kasiviswanathan
wrote:
> With a default pacing rate of 10, the uplink data rate for a single
> TCP stream is around 10Mbps. Setting it to 8 increases it to 146Mbps
> which is the maximum supported transmit rate.
>
> Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
> ---
> drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> index 8e1f43a..8f8c4f2 100644
> --- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> +++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_handlers.c
> @@ -16,6 +16,7 @@
> #include <linux/netdevice.h>
> #include <linux/netdev_features.h>
> #include <linux/if_arp.h>
> +#include <net/sock.h>
> #include "rmnet_private.h"
> #include "rmnet_config.h"
> #include "rmnet_vnd.h"
> @@ -204,6 +205,8 @@ void rmnet_egress_handler(struct sk_buff *skb)
> struct rmnet_priv *priv;
> u8 mux_id;
>
> + sk_pacing_shift_update(skb->sk, 8);
Well... Please tell us why this is needed in this driver.
This interface is meant for wifi aggregation, not to work around some
strange ethernet drivers designs.
^ permalink raw reply
* [PATCH net] ipv6: fix general protection fault in fib6_add()
From: Wei Wang @ 2018-01-03 22:11 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Martin KaFai Lau, David Ahern, Wei Wang
From: Wei Wang <weiwan@google.com>
In fib6_add(), pn could be NULL if fib6_add_1() failed to return a fib6
node. Checking pn != fn before accessing pn->leaf makes sure pn is not
NULL.
This fixes the following GPF reported by syzkaller:
general protection fault: 0000 [#1] SMP KASAN
Dumping ftrace buffer:
(ftrace buffer empty)
Modules linked in:
CPU: 0 PID: 3201 Comm: syzkaller001778 Not tainted 4.15.0-rc5+ #151
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
RIP: 0010:fib6_add+0x736/0x15a0 net/ipv6/ip6_fib.c:1244
RSP: 0018:ffff8801c7626a70 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: 0000000000000020 RCX: ffffffff84794465
RDX: 0000000000000004 RSI: ffff8801d38935f0 RDI: 0000000000000282
RBP: ffff8801c7626da0 R08: 1ffff10038ec4c35 R09: 0000000000000000
R10: ffff8801c7626c68 R11: 0000000000000000 R12: 00000000fffffffe
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000009
FS: 0000000000000000(0000) GS:ffff8801db200000(0063) knlGS:0000000009b70840
CS: 0010 DS: 002b ES: 002b CR0: 0000000080050033
CR2: 0000000020be1000 CR3: 00000001d585a006 CR4: 00000000001606f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__ip6_ins_rt+0x6c/0x90 net/ipv6/route.c:1006
ip6_route_multipath_add+0xd14/0x16c0 net/ipv6/route.c:3833
inet6_rtm_newroute+0xdc/0x160 net/ipv6/route.c:3957
rtnetlink_rcv_msg+0x733/0x1020 net/core/rtnetlink.c:4411
netlink_rcv_skb+0x21e/0x460 net/netlink/af_netlink.c:2408
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4423
netlink_unicast_kernel net/netlink/af_netlink.c:1275 [inline]
netlink_unicast+0x4e8/0x6f0 net/netlink/af_netlink.c:1301
netlink_sendmsg+0xa4a/0xe60 net/netlink/af_netlink.c:1864
sock_sendmsg_nosec net/socket.c:636 [inline]
sock_sendmsg+0xca/0x110 net/socket.c:646
sock_write_iter+0x31a/0x5d0 net/socket.c:915
call_write_iter include/linux/fs.h:1772 [inline]
do_iter_readv_writev+0x525/0x7f0 fs/read_write.c:653
do_iter_write+0x154/0x540 fs/read_write.c:932
compat_writev+0x225/0x420 fs/read_write.c:1246
do_compat_writev+0x115/0x220 fs/read_write.c:1267
C_SYSC_writev fs/read_write.c:1278 [inline]
compat_SyS_writev+0x26/0x30 fs/read_write.c:1274
do_syscall_32_irqs_on arch/x86/entry/common.c:327 [inline]
do_fast_syscall_32+0x3ee/0xf9d arch/x86/entry/common.c:389
entry_SYSENTER_compat+0x54/0x63 arch/x86/entry/entry_64_compat.S:125
Reported-by: syzbot <syzkaller@googlegroups.com>
Fixes: 66f5d6ce53e6 ("ipv6: replace rwlock with rcu and spinlock in fib6_table")
Signed-off-by: Wei Wang <weiwan@google.com>
---
net/ipv6/ip6_fib.c | 35 ++++++++++++++++++++---------------
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index f5285f4e1d08..d11a5578e4f8 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1241,23 +1241,28 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
* If fib6_add_1 has cleared the old leaf pointer in the
* super-tree leaf node we have to find a new one for it.
*/
- struct rt6_info *pn_leaf = rcu_dereference_protected(pn->leaf,
- lockdep_is_held(&table->tb6_lock));
- if (pn != fn && pn_leaf == rt) {
- pn_leaf = NULL;
- RCU_INIT_POINTER(pn->leaf, NULL);
- atomic_dec(&rt->rt6i_ref);
- }
- if (pn != fn && !pn_leaf && !(pn->fn_flags & RTN_RTINFO)) {
- pn_leaf = fib6_find_prefix(info->nl_net, table, pn);
-#if RT6_DEBUG >= 2
- if (!pn_leaf) {
- WARN_ON(!pn_leaf);
- pn_leaf = info->nl_net->ipv6.ip6_null_entry;
+ if (pn != fn) {
+ struct rt6_info *pn_leaf =
+ rcu_dereference_protected(pn->leaf,
+ lockdep_is_held(&table->tb6_lock));
+ if (pn_leaf == rt) {
+ pn_leaf = NULL;
+ RCU_INIT_POINTER(pn->leaf, NULL);
+ atomic_dec(&rt->rt6i_ref);
}
+ if (!pn_leaf && !(pn->fn_flags & RTN_RTINFO)) {
+ pn_leaf = fib6_find_prefix(info->nl_net, table,
+ pn);
+#if RT6_DEBUG >= 2
+ if (!pn_leaf) {
+ WARN_ON(!pn_leaf);
+ pn_leaf =
+ info->nl_net->ipv6.ip6_null_entry;
+ }
#endif
- atomic_inc(&pn_leaf->rt6i_ref);
- rcu_assign_pointer(pn->leaf, pn_leaf);
+ atomic_inc(&pn_leaf->rt6i_ref);
+ rcu_assign_pointer(pn->leaf, pn_leaf);
+ }
}
#endif
goto failure;
--
2.16.0.rc0.223.g4a4ac83678-goog
^ permalink raw reply related
* uapi/if_ether.h: prevent redefinition of struct ethhdr
From: Hauke Mehrtens @ 2018-01-03 22:14 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA,
musl-ZwoEplunGu1jrUoiu81ncdBPR1lH4CV8,
felix.janda-1KBjaw7Xf1+zQB+pC5nmwQ,
f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Hauke Mehrtens
Musl provides its own ethhdr struct definition. Add a guard to prevent
its definition of the appropriate musl header has already been included.
glibc does not implement this header, but when glibc will implement this
they can just define __UAPI_DEF_ETHHDR 0 to make it work with the
kernel.
Signed-off-by: Hauke Mehrtens <hauke-5/S+JYg5SzeELgA04lAiVw@public.gmane.org>
---
include/uapi/linux/if_ether.h | 3 +++
include/uapi/linux/libc-compat.h | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/include/uapi/linux/if_ether.h b/include/uapi/linux/if_ether.h
index 3ee3bf7c8526..144de4d2f385 100644
--- a/include/uapi/linux/if_ether.h
+++ b/include/uapi/linux/if_ether.h
@@ -23,6 +23,7 @@
#define _UAPI_LINUX_IF_ETHER_H
#include <linux/types.h>
+#include <linux/libc-compat.h>
/*
* IEEE 802.3 Ethernet magic constants. The frame sizes omit the preamble
@@ -149,11 +150,13 @@
* This is an Ethernet frame header.
*/
+#if __UAPI_DEF_ETHHDR
struct ethhdr {
unsigned char h_dest[ETH_ALEN]; /* destination eth addr */
unsigned char h_source[ETH_ALEN]; /* source ether addr */
__be16 h_proto; /* packet type ID field */
} __attribute__((packed));
+#endif
#endif /* _UAPI_LINUX_IF_ETHER_H */
diff --git a/include/uapi/linux/libc-compat.h b/include/uapi/linux/libc-compat.h
index 8254c937c9f4..fc29efaa918c 100644
--- a/include/uapi/linux/libc-compat.h
+++ b/include/uapi/linux/libc-compat.h
@@ -264,4 +264,10 @@
#endif /* __GLIBC__ */
+/* Definitions for if_ether.h */
+/* allow libcs like musl to deactivate this, glibc does not implement this. */
+#ifndef __UAPI_DEF_ETHHDR
+#define __UAPI_DEF_ETHHDR 1
+#endif
+
#endif /* _UAPI_LIBC_COMPAT_H */
--
2.11.0
^ permalink raw reply related
* Re: Linux ECN Handling
From: Steve Ibanez @ 2018-01-03 22:21 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, Yuchung Cheng, Daniel Borkmann, Netdev,
Florian Westphal, Mohammad Alizadeh, Lawrence Brakmo
In-Reply-To: <CADVnQym9c1n-=KTGou0_WJv-68XYaKG7BUuss9eXYg9aca1ffQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3169 bytes --]
Hi Neal,
I've attached a pdf of a slide that I made which shows some data from
the kernel log at the receiver as well as the timestamp, SeqNo, and
length of the corresponding segments from the tcpdump trace at the
receiver interface. Hopefully this helps clarify why I think
tcp_transmit_skb() is called at some point before tcp_ack_snd_check()
while processing the CWR segment for which no ACK is sent. Please let
me know if anything is unclear or if you know where that initial call
to tcp_transmit_skb() might be coming from and why it's happening
prematurely.
Best,
-Steve
On Wed, Jan 3, 2018 at 11:39 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Tue, Jan 2, 2018 at 6:57 PM, Steve Ibanez <sibanez@stanford.edu> wrote:
>> Hi Neal,
>>
>> Sorry, my last email was incorrect. It turns out the default tcp
>> congestion control alg that was being used on my client machines was
>> cubic instead of dctcp. That is why tp->processing_cwr field was never
>> set in the tcp_rcv_established function. I've changed the default back
>> to dctcp on all of my machines.
>>
>> I am now logging the value of tp->rcv_nxt at the top of the
>> tcp_transmit_skb() function for all CWR segments. I see that during
>> normal operation, the value of tp->rcv_nxt is equal to the SeqNo in
>> the CWR segment + length of the CWR segment.
>
> OK, thanks. That makes sense.
>
> This part I didn't understand:
>
>> However, for the unACKed
>> CWR segment, the value of tp->rcv_nxt is just equal to the SeqNo in
>> the CWR segment (i.e. not incremented by the length). And I see that
>> by the time the tcp_ack_snd_check() function is executed, tp->rcv_nxt
>> has been incremented by the length of the unACKed CWR segment.
>
> I would have thought that for the processing of the skb that has the
> CWR, the sequence would be:
>
> (1) "...the tcp_ack_snd_check() function is executed, tp->rcv_nxt has
> been incremented by the length of the unACKed CWR segment"
>
> (2) then we send the ACK, and the instrumentation at the top of the
> tcp_transmit_skb() function logs that rcv_nxt value (which "has been
> incremented by the length of the unACKed CWR segment").
>
> But you are saying "for the unACKed CWR segment, the value of
> tp->rcv_nxt is just equal to the SeqNo in the CWR segment (i.e. not
> incremented by the length)", which does not seem to match my
> prediction in (2). Apparently I am mis-understanding the sequence.
> Perhaps you can help clear it up for me? :-)
>
> Is it possible that the case where you see "tp->rcv_nxt is just equal
> to the SeqNo in the CWR segment" is a log line that was logged while
> processing the skb that precedes the skb with the CWR?
>
>> The tcp_transmit_skb() function sets the outgoing segment's ack_seq to
>> be tp->rcv_next:
>>
>> th->ack_seq = htonl(tp->rcv_nxt);
>>
>> So I think the rcv_nxt field is supposed to be incremented before
>> reaching tcp_transmit_skb(). Can you see any reason as to why this
>> field would not be incremented for CWR segments sometimes?
>
> No, so far I haven't been able to think of a reason why rcv_nxt would
> not be incremented for in-order CWR-marked segments...
>
> cheers,
> neal
[-- Attachment #2: CWR_log_and_trace.pdf --]
[-- Type: application/pdf, Size: 34236 bytes --]
^ permalink raw reply
* [PATCH] net/mlx5e: hide an unused variable
From: Arnd Bergmann @ 2018-01-03 22:40 UTC (permalink / raw)
To: Saeed Mahameed, Matan Barak, Leon Romanovsky
Cc: Arnd Bergmann, Or Gerlitz, Hadar Hen Zion, David S. Miller,
Paul Blakey, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
The uplink_rpriv variable was added at the start of the function but
only used inside of an #ifdef:
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 'mlx5e_route_lookup_ipv6':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:1549:25: error: unused variable 'uplink_rpriv' [-Werror=unused-variable]
This moves the declaration into that #ifdef as well.
Fixes: 5ed99fb421d4 ("net/mlx5e: Move ethernet representors data into separate struct")
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 25a8073f15d8..933275fe03b2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -1546,11 +1546,11 @@ static int mlx5e_route_lookup_ipv6(struct mlx5e_priv *priv,
struct neighbour **out_n,
int *out_ttl)
{
- struct mlx5e_rep_priv *uplink_rpriv;
struct neighbour *n = NULL;
struct dst_entry *dst;
#if IS_ENABLED(CONFIG_INET) && IS_ENABLED(CONFIG_IPV6)
+ struct mlx5e_rep_priv *uplink_rpriv;
struct mlx5_eswitch *esw = priv->mdev->priv.eswitch;
int ret;
--
2.9.0
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH net-next v2 05/10] net: qualcomm: rmnet: Set pacing rate
From: Subash Abhinov Kasiviswanathan @ 2018-01-03 22:45 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, netdev, lkp
In-Reply-To: <1515016918.131759.2.camel@gmail.com>
>> + sk_pacing_shift_update(skb->sk, 8);
>
> Well... Please tell us why this is needed in this driver.
>
> This interface is meant for wifi aggregation, not to work around some
> strange ethernet drivers designs.
Hi Eric
The real device over which the rmnet devices are installed also
aggregate multiple IP packets and sends them as a single large aggregate
frame to the hardware.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH net-next 1/4] l2tp: revert "l2tp: add peer_offset parameter"
From: James Chapman @ 2018-01-03 22:48 UTC (permalink / raw)
To: netdev; +Cc: g.nault, lorenzo.bianconi, liuhangbin, James Chapman
In-Reply-To: <1515019687-1556-1-git-send-email-jchapman@katalix.com>
Revert commit f15bc54eeecd ("l2tp: add peer_offset parameter"). This
is removed because it is adding another configurable offset and
configurable offsets are being removed.
Signed-off-by: James Chapman <jchapman@katalix.com>
---
include/uapi/linux/l2tp.h | 1 -
net/l2tp/l2tp_core.c | 3 +--
net/l2tp/l2tp_core.h | 13 +++----------
net/l2tp/l2tp_debugfs.c | 8 +++-----
net/l2tp/l2tp_netlink.c | 21 +--------------------
5 files changed, 8 insertions(+), 38 deletions(-)
diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h
index d6fee55..d84ce5c 100644
--- a/include/uapi/linux/l2tp.h
+++ b/include/uapi/linux/l2tp.h
@@ -127,7 +127,6 @@ enum {
L2TP_ATTR_UDP_ZERO_CSUM6_TX, /* flag */
L2TP_ATTR_UDP_ZERO_CSUM6_RX, /* flag */
L2TP_ATTR_PAD,
- L2TP_ATTR_PEER_OFFSET, /* u16 */
__L2TP_ATTR_MAX,
};
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 6ff6471..115918a 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -792,7 +792,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
ptr += 2 + offset;
}
} else
- ptr += session->peer_offset;
+ ptr += session->offset;
offset = ptr - optr;
if (!pskb_may_pull(skb, offset))
@@ -1785,7 +1785,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
session->lns_mode = cfg->lns_mode;
session->reorder_timeout = cfg->reorder_timeout;
session->offset = cfg->offset;
- session->peer_offset = cfg->peer_offset;
session->l2specific_type = cfg->l2specific_type;
session->l2specific_len = cfg->l2specific_len;
session->cookie_len = cfg->cookie_len;
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index c6fe7cc..9534e16 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -59,8 +59,7 @@ struct l2tp_session_cfg {
int debug; /* bitmask of debug message
* categories */
u16 vlan_id; /* VLAN pseudowire only */
- u16 offset; /* offset to tx payload */
- u16 peer_offset; /* offset to rx payload */
+ u16 offset; /* offset to payload */
u16 l2specific_len; /* Layer 2 specific length */
u16 l2specific_type; /* Layer 2 specific type */
u8 cookie[8]; /* optional cookie */
@@ -87,14 +86,8 @@ struct l2tp_session {
int cookie_len;
u8 peer_cookie[8];
int peer_cookie_len;
- u16 offset; /* offset from end of L2TP
- * header to beginning of
- * tx data
- */
- u16 peer_offset; /* offset from end of L2TP
- * header to beginning of
- * rx data
- */
+ u16 offset; /* offset from end of L2TP header
+ to beginning of data */
u16 l2specific_len;
u16 l2specific_type;
u16 hdr_len;
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index 4cc30b3..eb69411 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -180,9 +180,8 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
session->lns_mode ? "LNS" : "LAC",
session->debug,
jiffies_to_msecs(session->reorder_timeout));
- seq_printf(m, " offset %hu peer_offset %hu l2specific %hu/%hu\n",
- session->offset, session->peer_offset,
- session->l2specific_type, session->l2specific_len);
+ seq_printf(m, " offset %hu l2specific %hu/%hu\n",
+ session->offset, session->l2specific_type, session->l2specific_len);
if (session->cookie_len) {
seq_printf(m, " cookie %02x%02x%02x%02x",
session->cookie[0], session->cookie[1],
@@ -229,8 +228,7 @@ static int l2tp_dfs_seq_show(struct seq_file *m, void *v)
seq_puts(m, " debug tx-pkts/bytes/errs rx-pkts/bytes/errs\n");
seq_puts(m, " SESSION ID, peer ID, PWTYPE\n");
seq_puts(m, " refcnt cnt\n");
- seq_puts(m, " offset OFFSET peer_offset OFFSET");
- seq_puts(m, " l2specific TYPE/LEN\n");
+ seq_puts(m, " offset OFFSET l2specific TYPE/LEN\n");
seq_puts(m, " [ cookie ]\n");
seq_puts(m, " [ peer cookie ]\n");
seq_puts(m, " config mtu/mru/rcvseq/sendseq/dataseq/lns debug reorderto\n");
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index d7d4d7a..7e9c501 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -547,25 +547,9 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
}
if (tunnel->version > 2) {
- if (info->attrs[L2TP_ATTR_PEER_OFFSET]) {
- struct nlattr *peer_offset;
-
- peer_offset = info->attrs[L2TP_ATTR_PEER_OFFSET];
- cfg.peer_offset = nla_get_u16(peer_offset);
- }
-
- if (info->attrs[L2TP_ATTR_OFFSET]) {
+ if (info->attrs[L2TP_ATTR_OFFSET])
cfg.offset = nla_get_u16(info->attrs[L2TP_ATTR_OFFSET]);
- /* in order to maintain compatibility with older
- * versions where offset was used for both tx and
- * rx side, update rx side with offset if peer_offset
- * is not provided by userspace
- */
- if (!info->attrs[L2TP_ATTR_PEER_OFFSET])
- cfg.peer_offset = cfg.offset;
- }
-
if (info->attrs[L2TP_ATTR_DATA_SEQ])
cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]);
@@ -779,8 +763,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
nla_put_string(skb, L2TP_ATTR_IFNAME, session->ifname)) ||
(session->offset &&
nla_put_u16(skb, L2TP_ATTR_OFFSET, session->offset)) ||
- (session->peer_offset &&
- nla_put_u16(skb, L2TP_ATTR_PEER_OFFSET, session->peer_offset)) ||
(session->cookie_len &&
nla_put(skb, L2TP_ATTR_COOKIE, session->cookie_len,
&session->cookie[0])) ||
@@ -921,7 +903,6 @@ static int l2tp_nl_cmd_session_dump(struct sk_buff *skb, struct netlink_callback
[L2TP_ATTR_PW_TYPE] = { .type = NLA_U16, },
[L2TP_ATTR_ENCAP_TYPE] = { .type = NLA_U16, },
[L2TP_ATTR_OFFSET] = { .type = NLA_U16, },
- [L2TP_ATTR_PEER_OFFSET] = { .type = NLA_U16, },
[L2TP_ATTR_DATA_SEQ] = { .type = NLA_U8, },
[L2TP_ATTR_L2SPEC_TYPE] = { .type = NLA_U8, },
[L2TP_ATTR_L2SPEC_LEN] = { .type = NLA_U8, },
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 3/4] l2tp: remove configurable payload offset
From: James Chapman @ 2018-01-03 22:48 UTC (permalink / raw)
To: netdev; +Cc: g.nault, lorenzo.bianconi, liuhangbin, James Chapman
In-Reply-To: <1515019687-1556-1-git-send-email-jchapman@katalix.com>
If L2TP_ATTR_OFFSET is set to a non-zero value in L2TPv3 tunnels, it
results in L2TPv3 packets being transmitted which might not be
compliant with the L2TPv3 RFC. This patch has l2tp ignore the offset
setting and send all packets with no offset.
In more detail:
L2TPv2 supports a variable offset from the L2TPv2 header to the
payload. The offset value is indicated by an optional field in the
L2TP header. Our L2TP implementation already detects the presence of
the optional offset and skips that many bytes when handling data
received packets. All transmitted packets are always transmitted with
no offset.
L2TPv3 has no optional offset field in the L2TPv3 packet
header. Instead, L2TPv3 defines optional fields in a "Layer-2 Specific
Sublayer". At the time when the original L2TP code was written, there
was talk at IETF of offset being implemented in a new Layer-2 Specific
Sublayer. A L2TP_ATTR_OFFSET netlink attribute was added so that this
offset could be configured and the intention was to allow it to be
also used to set the tx offset for L2TPv2. However, no L2TPv3 offset
was ever specified and the L2TP_ATTR_OFFSET parameter was forgotten
about.
Setting L2TP_ATTR_OFFSET results in L2TPv3 packets being transmitted
with the specified number of bytes padding between L2TPv3 header and
payload. This is not compliant with L2TPv3 RFC3931. This change
removes the configurable offset altogether while retaining
L2TP_ATTR_OFFSET for backwards compatibility. Any L2TP_ATTR_OFFSET
value is ignored.
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_core.c | 14 ++++----------
net/l2tp/l2tp_core.h | 3 ---
net/l2tp/l2tp_debugfs.c | 4 ++--
net/l2tp/l2tp_netlink.c | 3 ---
4 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index 115918a..786cd7f 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -780,10 +780,8 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
}
}
- /* Session data offset is handled differently for L2TPv2 and
- * L2TPv3. For L2TPv2, there is an optional 16-bit value in
- * the header. For L2TPv3, the offset is negotiated using AVPs
- * in the session setup control protocol.
+ /* Session data offset is defined only for L2TPv2 and is
+ * indicated by an optional 16-bit value in the header.
*/
if (tunnel->version == L2TP_HDR_VER_2) {
/* If offset bit set, skip it. */
@@ -791,8 +789,7 @@ void l2tp_recv_common(struct l2tp_session *session, struct sk_buff *skb,
offset = ntohs(*(__be16 *)ptr);
ptr += 2 + offset;
}
- } else
- ptr += session->offset;
+ }
offset = ptr - optr;
if (!pskb_may_pull(skb, offset))
@@ -1068,8 +1065,6 @@ static int l2tp_build_l2tpv3_header(struct l2tp_session *session, void *buf)
}
bufp += session->l2specific_len;
}
- if (session->offset)
- bufp += session->offset;
return bufp - optr;
}
@@ -1734,7 +1729,7 @@ void l2tp_session_set_header_len(struct l2tp_session *session, int version)
if (session->send_seq)
session->hdr_len += 4;
} else {
- session->hdr_len = 4 + session->cookie_len + session->l2specific_len + session->offset;
+ session->hdr_len = 4 + session->cookie_len + session->l2specific_len;
if (session->tunnel->encap == L2TP_ENCAPTYPE_UDP)
session->hdr_len += 4;
}
@@ -1784,7 +1779,6 @@ struct l2tp_session *l2tp_session_create(int priv_size, struct l2tp_tunnel *tunn
session->recv_seq = cfg->recv_seq;
session->lns_mode = cfg->lns_mode;
session->reorder_timeout = cfg->reorder_timeout;
- session->offset = cfg->offset;
session->l2specific_type = cfg->l2specific_type;
session->l2specific_len = cfg->l2specific_len;
session->cookie_len = cfg->cookie_len;
diff --git a/net/l2tp/l2tp_core.h b/net/l2tp/l2tp_core.h
index 9534e16..c2e9bbd 100644
--- a/net/l2tp/l2tp_core.h
+++ b/net/l2tp/l2tp_core.h
@@ -59,7 +59,6 @@ struct l2tp_session_cfg {
int debug; /* bitmask of debug message
* categories */
u16 vlan_id; /* VLAN pseudowire only */
- u16 offset; /* offset to payload */
u16 l2specific_len; /* Layer 2 specific length */
u16 l2specific_type; /* Layer 2 specific type */
u8 cookie[8]; /* optional cookie */
@@ -86,8 +85,6 @@ struct l2tp_session {
int cookie_len;
u8 peer_cookie[8];
int peer_cookie_len;
- u16 offset; /* offset from end of L2TP header
- to beginning of data */
u16 l2specific_len;
u16 l2specific_type;
u16 hdr_len;
diff --git a/net/l2tp/l2tp_debugfs.c b/net/l2tp/l2tp_debugfs.c
index eb69411..2c30587 100644
--- a/net/l2tp/l2tp_debugfs.c
+++ b/net/l2tp/l2tp_debugfs.c
@@ -180,8 +180,8 @@ static void l2tp_dfs_seq_session_show(struct seq_file *m, void *v)
session->lns_mode ? "LNS" : "LAC",
session->debug,
jiffies_to_msecs(session->reorder_timeout));
- seq_printf(m, " offset %hu l2specific %hu/%hu\n",
- session->offset, session->l2specific_type, session->l2specific_len);
+ seq_printf(m, " offset 0 l2specific %hu/%hu\n",
+ session->l2specific_type, session->l2specific_len);
if (session->cookie_len) {
seq_printf(m, " cookie %02x%02x%02x%02x",
session->cookie[0], session->cookie[1],
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index a1f24fb..e1ca29f 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -547,9 +547,6 @@ static int l2tp_nl_cmd_session_create(struct sk_buff *skb, struct genl_info *inf
}
if (tunnel->version > 2) {
- if (info->attrs[L2TP_ATTR_OFFSET])
- cfg.offset = nla_get_u16(info->attrs[L2TP_ATTR_OFFSET]);
-
if (info->attrs[L2TP_ATTR_DATA_SEQ])
cfg.data_seq = nla_get_u8(info->attrs[L2TP_ATTR_DATA_SEQ]);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 2/4] l2tp: revert "l2tp: fix missing print session offset info"
From: James Chapman @ 2018-01-03 22:48 UTC (permalink / raw)
To: netdev; +Cc: g.nault, lorenzo.bianconi, liuhangbin, James Chapman
In-Reply-To: <1515019687-1556-1-git-send-email-jchapman@katalix.com>
Revert commit 820da5357572 ("l2tp: fix missing print session offset
info"). The peer_offset parameter is removed.
Signed-off-by: James Chapman <jchapman@katalix.com>
---
net/l2tp/l2tp_netlink.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 7e9c501..a1f24fb 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -761,8 +761,6 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
if ((session->ifname[0] &&
nla_put_string(skb, L2TP_ATTR_IFNAME, session->ifname)) ||
- (session->offset &&
- nla_put_u16(skb, L2TP_ATTR_OFFSET, session->offset)) ||
(session->cookie_len &&
nla_put(skb, L2TP_ATTR_COOKIE, session->cookie_len,
&session->cookie[0])) ||
--
1.9.1
^ permalink raw reply related
* [PATCH net-next 0/4] l2tp: remove configurable offset parameters
From: James Chapman @ 2018-01-03 22:48 UTC (permalink / raw)
To: netdev; +Cc: g.nault, lorenzo.bianconi, liuhangbin, James Chapman
This patch series removes all code to support a configurable offset in
transmitted l2tp packets. Code to handle this is incomplete and buggy
and has been this way for years. If anyone tried to configure an
offset, it would be ignored for L2TPv2 tunnels, or for L2TPv3 tunnels,
could result in L2TPv3 packets being transmitted which are not
compliant with L2TPv3 RFC3931. This patch series removes the support
for configurable offsets.
No known userspace l2tp daemon configures an offset. However,
iproute2's "ip l2tp" command has an offset parameter and if set, the
value is passed to the kernel. This is the most likely use case where
offsets might be configured, e.g.
ip l2tp add tunnel local 1.1.1.1 remote 1.1.1.2 tunnel_id 1 \
peer_tunnel_id 2 encap ip
ip l2tp add session name l2tp0 tunnel_id 1 session_id 1 \
peer_session_id 2 offset 8
The above would result in packets being transmitted to 1.1.1.2 with 8
bytes padding between the L2TPv3 header and the payload. The peer
would need to be configured with the same offset value. However, the
packets are not compliant with the L2TPv3 RFC, hence I think it's
unlikely that offset is being used. With this patch series applied,
the offset would not be configured. The peer would need to be modified to
remove its offset setting too.
iproute2 should be modified to remove or ignore the ip l2tp offset
parameter.
This issue was discovered when reviewing a patch series from
lorenzo.bianconi@redhat.com which adds another netlink attribute to
configure the expected offset in received L2TPv3 packets. This change
is reverted by this series because offsets do not exist in L2TPv3
packets. These commits are:
commit f15bc54eeecd ("l2tp: add peer_offset parameter")
commit 820da5357572 ("l2tp: fix missing print session offset info")
In more detail:
The L2TPv2 protocol supports a variable offset from the L2TPv2 header
to the payload to give the sender implementation some flexibility for
data alignment when adding L2TP headers on to payloads. The offset
value is indicated by an optional field in the L2TP header. Our L2TP
implementation already detects the presence of the optional offset in
received packets and skips those bytes when parsing packets. All
transmitted L2TPv2 packets are always transmitted with no offset.
L2TPv3 has no optional offset field in the L2TPv3 packet
header. Instead, L2TPv3 defines optional fields in a "Layer-2 Specific
Sublayer". At the time when the original L2TP code was written, there
was talk at IETF of offset being implemented in a new Layer-2 Specific
Sublayer. A L2TP_ATTR_OFFSET netlink attribute was added so that this
offset could be configured and the intention was to allow it to be
also used to set the tx offset for L2TPv2. However, no L2TPv3 offset
was ever specified and the L2TP_ATTR_OFFSET parameter was forgotten
about.
Setting L2TP_ATTR_OFFSET results in L2TPv3 packets being transmitted
with the specified number of bytes padding between L2TPv3 header and
payload. This is not compliant with L2TPv3 RFC3931. So this change
removes the configurable offset altogether while retaining
L2TP_ATTR_OFFSET in the API for backwards compatibility. If
L2TP_ATTR_OFFSET is given, its value is now silently ignored.
James Chapman (4):
l2tp: revert "l2tp: add peer_offset parameter"
l2tp: revert "l2tp: fix missing print session offset info"
l2tp: remove configurable payload offset
l2tp: add comment in API header that L2TP_ATTR_OFFSET is not used
include/uapi/linux/l2tp.h | 3 +--
net/l2tp/l2tp_core.c | 15 ++++-----------
net/l2tp/l2tp_core.h | 10 ----------
net/l2tp/l2tp_debugfs.c | 6 ++----
net/l2tp/l2tp_netlink.c | 24 ------------------------
5 files changed, 7 insertions(+), 51 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next 4/4] l2tp: add comment in API header that L2TP_ATTR_OFFSET is not used
From: James Chapman @ 2018-01-03 22:48 UTC (permalink / raw)
To: netdev; +Cc: g.nault, lorenzo.bianconi, liuhangbin, James Chapman
In-Reply-To: <1515019687-1556-1-git-send-email-jchapman@katalix.com>
Signed-off-by: James Chapman <jchapman@katalix.com>
---
include/uapi/linux/l2tp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h
index d84ce5c..f78eef4 100644
--- a/include/uapi/linux/l2tp.h
+++ b/include/uapi/linux/l2tp.h
@@ -94,7 +94,7 @@ enum {
L2TP_ATTR_NONE, /* no data */
L2TP_ATTR_PW_TYPE, /* u16, enum l2tp_pwtype */
L2TP_ATTR_ENCAP_TYPE, /* u16, enum l2tp_encap_type */
- L2TP_ATTR_OFFSET, /* u16 */
+ L2TP_ATTR_OFFSET, /* u16 (not used) */
L2TP_ATTR_DATA_SEQ, /* u16 */
L2TP_ATTR_L2SPEC_TYPE, /* u8, enum l2tp_l2spec_type */
L2TP_ATTR_L2SPEC_LEN, /* u8, enum l2tp_l2spec_type */
--
1.9.1
^ permalink raw reply related
* [PATCH net v2 0/2] SCTP PMTU discovery fixes
From: Marcelo Ricardo Leitner @ 2018-01-03 22:59 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, Xin Long, Vlad Yasevich, Neil Horman
This patchset fixes 2 issues with PMTU discovery that can lead to flood
of retransmissions.
The first patch fixes the issue for when PMTUD is disabled by the
application, while the second fixes it for when its enabled.
Please consider these to stable.
Thanks,
Marcelo Ricardo Leitner (2):
sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
sctp: fix the handling of ICMP Frag Needed for too small MTUs
include/net/sctp/structs.h | 2 +-
net/sctp/input.c | 21 +++++++++++++++------
net/sctp/transport.c | 29 +++++++++++++++++++----------
3 files changed, 35 insertions(+), 17 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH net v2 1/2] sctp: do not retransmit upon FragNeeded if PMTU discovery is disabled
From: Marcelo Ricardo Leitner @ 2018-01-03 22:59 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, Xin Long, Vlad Yasevich, Neil Horman
In-Reply-To: <cover.1515020059.git.marcelo.leitner@gmail.com>
Currently, if PMTU discovery is disabled on a given transport, but the
configured value is higher than the actual PMTU, it is likely that we
will get some icmp Frag Needed. The issue is, if PMTU discovery is
disabled, we won't update the information and will issue a
retransmission immediately, which may very well trigger another ICMP,
and another retransmission, leading to a loop.
The fix is to simply not trigger immediate retransmissions if PMTU
discovery is disabled on the given transport.
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
net/sctp/input.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 621b5ca3fd1c17c3d7ef7bb1c7677ab98cebbe77..4a8e76f4834c90de9398455862423e598b8354a7 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -399,13 +399,18 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
return;
}
- if (t->param_flags & SPP_PMTUD_ENABLE) {
- /* Update transports view of the MTU */
- sctp_transport_update_pmtu(t, pmtu);
+ if (!(t->param_flags & SPP_PMTUD_ENABLE))
+ /* We can't allow retransmitting in such case, as the
+ * retransmission would be sized just as before, and thus we
+ * would get another icmp, and retransmit again.
+ */
+ return;
- /* Update association pmtu. */
- sctp_assoc_sync_pmtu(asoc);
- }
+ /* Update transports view of the MTU */
+ sctp_transport_update_pmtu(t, pmtu);
+
+ /* Update association pmtu. */
+ sctp_assoc_sync_pmtu(asoc);
/* Retransmit with the new pmtu setting.
* Normally, if PMTU discovery is disabled, an ICMP Fragmentation
--
2.14.3
^ permalink raw reply related
* [PATCH net v2 2/2] sctp: fix the handling of ICMP Frag Needed for too small MTUs
From: Marcelo Ricardo Leitner @ 2018-01-03 22:59 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, Xin Long, Vlad Yasevich, Neil Horman
In-Reply-To: <cover.1515020059.git.marcelo.leitner@gmail.com>
syzbot reported a hang involving SCTP, on which it kept flooding dmesg
with the message:
[ 246.742374] sctp: sctp_transport_update_pmtu: Reported pmtu 508 too
low, using default minimum of 512
That happened because whenever SCTP hits an ICMP Frag Needed, it tries
to adjust to the new MTU and triggers an immediate retransmission. But
it didn't consider the fact that MTUs smaller than the SCTP minimum MTU
allowed (512) would not cause the PMTU to change, and issued the
retransmission anyway (thus leading to another ICMP Frag Needed, and so
on).
As IPv4 (ip_rt_min_pmtu=556) and IPv6 (IPV6_MIN_MTU=1280) minimum MTU
are higher than that, sctp_transport_update_pmtu() is changed to
re-fetch the PMTU that got set after our request, and with that, detect
if there was an actual change or not.
The fix, thus, skips the immediate retransmission if the received ICMP
resulted in no change, in the hope that SCTP will select another path.
Note: The value being used for the minimum MTU (512,
SCTP_DEFAULT_MINSEGMENT) is not right and instead it should be (576,
SCTP_MIN_PMTU), but such change belongs to another patch.
Changes from v1:
- do not disable PMTU discovery, in the light of commit
06ad391919b2 ("[SCTP] Don't disable PMTU discovery when mtu is small")
and as suggested by Xin Long.
- changed the way to break the rtx loop by detecting if the icmp
resulted in a change or not.
- change the warn to use pr_warn_ratelimited, to protect in case there
are several transports using such path.
See-also: https://lkml.org/lkml/2017/12/22/811
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
include/net/sctp/structs.h | 2 +-
net/sctp/input.c | 8 ++++++--
net/sctp/transport.c | 29 +++++++++++++++++++----------
3 files changed, 26 insertions(+), 13 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 2f8f93da5dc2660f4db37c04f8a434809b3120a1..9a5ccf03a59b1e0a4820e2a978c66f1df8a9a82f 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -966,7 +966,7 @@ void sctp_transport_burst_limited(struct sctp_transport *);
void sctp_transport_burst_reset(struct sctp_transport *);
unsigned long sctp_transport_timeout(struct sctp_transport *);
void sctp_transport_reset(struct sctp_transport *t);
-void sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu);
+bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu);
void sctp_transport_immediate_rtx(struct sctp_transport *);
void sctp_transport_dst_release(struct sctp_transport *t);
void sctp_transport_dst_confirm(struct sctp_transport *t);
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 4a8e76f4834c90de9398455862423e598b8354a7..3cddbecabaa2db410971c66d3256c466c7f5289f 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -406,8 +406,12 @@ void sctp_icmp_frag_needed(struct sock *sk, struct sctp_association *asoc,
*/
return;
- /* Update transports view of the MTU */
- sctp_transport_update_pmtu(t, pmtu);
+ /* Update transports view of the MTU. Return if no update was needed.
+ * If an update wasn't needed/possible, it also doesn't make sense to
+ * try to retransmit now.
+ */
+ if (!sctp_transport_update_pmtu(t, pmtu))
+ return;
/* Update association pmtu. */
sctp_assoc_sync_pmtu(asoc);
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index 1e5a22430cf56e40a6f323081beb97836b506384..47f82bd794d915188bad037463c2aa14175a55ef 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -248,28 +248,37 @@ void sctp_transport_pmtu(struct sctp_transport *transport, struct sock *sk)
transport->pathmtu = SCTP_DEFAULT_MAXSEGMENT;
}
-void sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
+bool sctp_transport_update_pmtu(struct sctp_transport *t, u32 pmtu)
{
struct dst_entry *dst = sctp_transport_dst_check(t);
+ bool change = true;
if (unlikely(pmtu < SCTP_DEFAULT_MINSEGMENT)) {
- pr_warn("%s: Reported pmtu %d too low, using default minimum of %d\n",
- __func__, pmtu, SCTP_DEFAULT_MINSEGMENT);
- /* Use default minimum segment size and disable
- * pmtu discovery on this transport.
- */
- t->pathmtu = SCTP_DEFAULT_MINSEGMENT;
- } else {
- t->pathmtu = pmtu;
+ pr_warn_ratelimited("%s: Reported pmtu %d too low, using default minimum of %d\n",
+ __func__, pmtu, SCTP_DEFAULT_MINSEGMENT);
+ /* Use default minimum segment instead */
+ pmtu = SCTP_DEFAULT_MINSEGMENT;
}
+ pmtu = SCTP_TRUNC4(pmtu);
if (dst) {
dst->ops->update_pmtu(dst, t->asoc->base.sk, NULL, pmtu);
dst = sctp_transport_dst_check(t);
}
- if (!dst)
+ if (!dst) {
t->af_specific->get_dst(t, &t->saddr, &t->fl, t->asoc->base.sk);
+ dst = t->dst;
+ }
+
+ if (dst) {
+ /* Re-fetch, as under layers may have a higher minimum size */
+ pmtu = SCTP_TRUNC4(dst_mtu(dst));
+ change = t->pathmtu != pmtu;
+ }
+ t->pathmtu = pmtu;
+
+ return change;
}
/* Caches the dst entry and source address for a transport's destination
--
2.14.3
^ permalink raw reply related
* Re: [RFC PATCH net-next 03/19] ipv6: Clear nexthop flags upon netdev up
From: David Ahern @ 2018-01-03 23:08 UTC (permalink / raw)
To: Ido Schimmel; +Cc: Ido Schimmel, netdev, davem, roopa, nicolas.dichtel, mlxsw
In-Reply-To: <20180103205321.GA13248@splinter>
On 1/3/18 1:53 PM, Ido Schimmel wrote:
> On Wed, Jan 03, 2018 at 11:47:16AM -0700, David Ahern wrote:
>> On 1/3/18 10:40 AM, Ido Schimmel wrote:
>>> David, can we please get back to the issue at hand? What's the problem
>>> with the location of the call to rt6_sync_up()?
>>
>> My original comment was asking why do it on NETDEV_CHANGE when it should
>> only be needed on NETDEV_UP.
>
> I can condition the call to rt6_sync_up() on the event being NETDEV_UP,
> but the location needs to stay the same. Before that the interface still
> doesn't have an IP address.
it's fine as is. The NETDEV_CHANGE section has a break after calling
rt6_sync_up(RTNH_F_LINKDOWN) which is added later. To your point,
addrconf_dev_config will add a linklocal address.
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Write in tcp_v6_syn_recv_sock
From: Cong Wang @ 2018-01-03 23:31 UTC (permalink / raw)
To: Ozgur
Cc: syzbot, David Miller, Alexey Kuznetsov, LKML,
Linux Kernel Network Developers, syzkaller-bugs@googlegroups.com,
Hideaki YOSHIFUJI, avejwatson@fb.com, ilyal@mellanox.com,
aviadye@mellanox.com
In-Reply-To: <681471515012954@web19j.yandex.ru>
On Wed, Jan 3, 2018 at 12:55 PM, Ozgur <ozgur@goosey.org> wrote:
>
>
> 03.01.2018, 21:57, "Cong Wang" <xiyou.wangcong@gmail.com>:
>> On Tue, Jan 2, 2018 at 3:58 PM, syzbot
>> <syzbot+6dc95bddc6976b800b0b@syzkaller.appspotmail.com> wrote:
>>> Hello,
>>>
>>> syzkaller hit the following crash on
>>> 61233580f1f33c50e159c50e24d80ffd2ba2e06b
>>> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
>>> compiler: gcc (GCC) 7.1.1 20170620
>>> .config is attached
>>> Raw console output is attached.
>>> C reproducer is attached
>>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>>> for information about syzkaller reproducers
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: syzbot+6dc95bddc6976b800b0b@syzkaller.appspotmail.com
>>> It will help syzbot understand when the bug is fixed. See footer for
>>> details.
>>> If you forward the report, please keep this part and the footer.
>>>
>>> TCP: request_sock_TCPv6: Possible SYN flooding on port 20002. Sending
>>> cookies. Check SNMP counters.
>>> ==================================================================
>>> BUG: KASAN: slab-out-of-bounds in memcpy include/linux/string.h:344 [inline]
>>> BUG: KASAN: slab-out-of-bounds in tcp_v6_syn_recv_sock+0x628/0x23a0
>>> net/ipv6/tcp_ipv6.c:1144
>>> Write of size 160 at addr ffff8801cbdd7460 by task syzkaller545407/3196
>>>
>>> CPU: 1 PID: 3196 Comm: syzkaller545407 Not tainted 4.15.0-rc5+ #241
>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>> Google 01/01/2011
>>> Call Trace:
>>> <IRQ>
>>> __dump_stack lib/dump_stack.c:17 [inline]
>>> dump_stack+0x194/0x257 lib/dump_stack.c:53
>>> print_address_description+0x73/0x250 mm/kasan/report.c:252
>>> kasan_report_error mm/kasan/report.c:351 [inline]
>>> kasan_report+0x25b/0x340 mm/kasan/report.c:409
>>> check_memory_region_inline mm/kasan/kasan.c:260 [inline]
>>> check_memory_region+0x137/0x190 mm/kasan/kasan.c:267
>>> memcpy+0x37/0x50 mm/kasan/kasan.c:303
>>> memcpy include/linux/string.h:344 [inline]
>>> tcp_v6_syn_recv_sock+0x628/0x23a0 net/ipv6/tcp_ipv6.c:1144
>>
>> tls_init() changes sk->sk_prot from IPv6 to IPv4, which leads
>> to this bug. I guess IPv6 is not supported for TLS? If so, need
>> a check on proto in tls_init()...
>
> Hello,
>
> I think IPv6 supports with TLS.
> There was a previously posted commit by Mellanox:
>
> https://patchwork.ozlabs.org/patch/801530/
Good to know.
Can you resend the fix? It could probably fix another warning
reported by syzbot too.
^ permalink raw reply
* Re: "lockless" qdisc breaks tx_queue_len change too?
From: Cong Wang @ 2018-01-03 23:41 UTC (permalink / raw)
To: John Fastabend; +Cc: Linux Kernel Network Developers
In-Reply-To: <cb5e1c62-9f71-b6ef-8a67-d094ce5c8f16@gmail.com>
On Wed, Jan 3, 2018 at 10:09 AM, John Fastabend
<john.fastabend@gmail.com> wrote:
> On 01/02/2018 08:41 PM, Cong Wang wrote:
>> Hi, John
>>
>> While reviewing your ptr_ring fix again today, it looks like your
>> "lockless" qdisc patchset breaks dev->tx_queue_len behavior.
>>
>> Before your patchset, dev->tx_queue_len is merely an integer to read,
>> after your patchset, the skb array has to be resized when
>> dev->tx_queue_len changes, but I don't see any qdisc code handles
>> this...
>>
>> Also, because of that, I doubt __skb_array_empty() in
>> pfifo_fast_dequeue() can be safe any more even with your ptr_ring fix.
>>
>> What am I missing?
>>
>
> I dropped support for tx_queue_len changes after qdisc has been
> created. The only check is at init time when building the qdisc.
This is where it breaks.
>
> Before this series teql and pfifo_fast were the only qdiscs that
> used tx_queue_len other qdiscs used other mechanisms or copied
> tx_queue_len at init time. So the API is inconsistent.
Yeah, pfifo_fast was able to drop based on latest value of tx_queue_len
before your patchset, this is why I am complaining.
>
> OK, but arguably its kAPI now and needs to be supported on live
> qdiscs. So couple options drop the __skb_array_empty() check,
> stop supporting changes on running qdiscs, or do a qdisc swap
> with the new array.
I don't think we can break the old behavior of tx_queue_len change
for pfifo_fast, people may already rely on it.
Doing a swap seems reasonable.
>
> I'm tempted to make the qdisc swap work, still need benchmarks
> I guess without the empty check. Either way to get it working
> we need a callback from tx_queue_len code paths.
Right, probably need a new ops in Qdisc_ops.
>
> Unfortunately, I guess someone somewhere probably uses pfifo_fast
> and changes there queue length with a script after creating the
> qdisc and expects it to work.
>
This is my concern as well. I will work on some patches, this doesn't
look trivial to solve at all.
Thanks.
^ permalink raw reply
* Re: WARNING in sk_stream_kill_queues (2)
From: Cong Wang @ 2018-01-03 23:44 UTC (permalink / raw)
To: syzbot
Cc: David Miller, Greg KH, Kate Stewart, LKML, Ingo Molnar,
Linux Kernel Network Developers, Philippe Ombredanne,
syzkaller-bugs, Thomas Gleixner
In-Reply-To: <94eb2c18a1d87f79230561d3de2c@google.com>
#syz dup: KASAN: slab-out-of-bounds Write in tcp_v6_syn_recv_sock
^ permalink raw reply
* Re: [patch net-next v4 00/10] net: sched: allow qdiscs to share filter block instances
From: Jakub Kicinski @ 2018-01-03 23:51 UTC (permalink / raw)
To: Jiri Pirko
Cc: David Ahern, netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew,
vivien.didelot, f.fainelli, michael.chan, ganeshgr, saeedm,
matanb, leonro, idosch, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <20180103172209.GD2067@nanopsycho.orion>
On Wed, 3 Jan 2018 18:22:09 +0100, Jiri Pirko wrote:
> However I don't agree about breaking the existing filter add and show
> and also imposibility to make not-shared block shared in the runtime
> before defining it first.
FWIW I would agree with David that allowing add on a shared block
modify filters on another interface can break existing users. (No
opinion on dump and lifetime).
^ permalink raw reply
* Re: [PATCH bpf-next v4 1/3] libbpf: add function to setup XDP
From: Eric Leblond @ 2018-01-03 23:59 UTC (permalink / raw)
To: daniel, Toshiaki Makita, Philippe Ombredanne
Cc: Alexei Starovoitov, netdev, linux-kernel
In-Reply-To: <20171230204116.30871-2-eric@regit.org>
Hello,
On Sat, 2017-12-30 at 21:41 +0100, Eric Leblond wrote:
> Most of the code is taken from set_link_xdp_fd() in bpf_load.c and
> slightly modified to be library compliant.
I've just discovered this patch is breaking the build of samples/bpf/
(nlattr not included at least and some int type problem). I'm going to
resubmit a patchset fixing this.
Sorry for the noise.
Best regards,
--
Eric Leblond <eric@regit.org>
Blog: https://home.regit.org/
^ permalink raw reply
* Re: [patch net-next v2 00/10] Add support for resource abstraction
From: Arkadi Sharshevsky @ 2018-01-04 0:07 UTC (permalink / raw)
To: David Ahern, Jiri Pirko
Cc: netdev, roopa, davem, mlxsw, andrew, vivien.didelot, f.fainelli,
michael.chan, ganeshgr, saeedm, matanb, leonro, idosch,
jakub.kicinski, ast, daniel, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, linville, gospo, steven.lin1,
yuvalm, ogerlitz
In-Reply-To: <49c72225-6437-54d5-a046-96fff5b65ce9@cumulusnetworks.com>
On 01/03/2018 08:29 PM, David Ahern wrote:
> On 1/3/18 11:17 AM, Jiri Pirko wrote:
>> Wed, Jan 03, 2018 at 07:14:16PM CET, dsa@cumulusnetworks.com wrote:
>>> On 1/3/18 11:05 AM, Arkadi Sharshevsky wrote:
>>>> As I stated this is a user-space bug which I fixed, and updated my repo
>>>> so please pull. Devlink uses mnl,and currently mnl does not support
>>>> extended ack. I added support for this in my local ver of libmnl:
>>>>
>>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Farkadis%2Flibmnl.git&data=02%7C01%7Carkadis%40mellanox.com%7C5c86b6240eb84459c6ae08d552d7f9a4%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636506009929977440&sdata=sgrNzMhPwe63BIVxexZTjl%2FXqW51kpuRiHVhTDNaa70%3D&reserved=0
>>>>
>>>> On branch master, so you can check it out. Besides this bugs, which were
>>>> userspace, can please specify what are the pending problems from your
>>>> point of view? Thanks!
>>>
>>> devlink is in iproute2 package and it has extack support. See 'git log
>>> lib/libnetlink.c'
>>
>> Dave, devlink uses libmnl.
>>
>
> Now I remember. You wrote it independently and but needed iproute2 be a
> delivery vehicle. It uses none of the common infrastructure from
> iproute2. Could we make this more difficult ....
>
> Sometime in the next day I will jump through the hoops to get a proper
> devlink command.
>
This actually was very confusing, I think the extack should be
handled by libmnl and iproute should use mnl_cb_run() routines
and not to implement its own. That way we could both benefit
from that.
You actually do use libmnl in libnetlink.c only for parsing
the headers, and its a dependency for extack handling.
I see this as a completely independent user space issue, which
doesn't have to do anything with this patchset. Not to mention
that everything is working right now.
^ permalink raw reply
* Re: [PATCH net-next 0/2] Enable virtio to act as a master for a passthru device
From: Samudrala, Sridhar @ 2018-01-04 0:22 UTC (permalink / raw)
To: Alexander Duyck
Cc: Jakub Kicinski, Brandeburg, Jesse, Michael S. Tsirkin,
Stephen Hemminger, Netdev, virtualization, virtio-dev,
Alexander Duyck
In-Reply-To: <CAKgT0UdtHVq3WWpjHggNabw_+2piaRQAKguivV690HdzKrg62w@mail.gmail.com>
On 1/3/2018 10:28 AM, Alexander Duyck wrote:
> On Wed, Jan 3, 2018 at 10:14 AM, Samudrala, Sridhar
> <sridhar.samudrala@intel.com> wrote:
>>
>> On 1/3/2018 8:59 AM, Alexander Duyck wrote:
>>> On Tue, Jan 2, 2018 at 6:16 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
>>>> On Tue, 2 Jan 2018 16:35:36 -0800, Sridhar Samudrala wrote:
>>>>> This patch series enables virtio to switch over to a VF datapath when a
>>>>> VF
>>>>> netdev is present with the same MAC address. It allows live migration of
>>>>> a VM
>>>>> with a direct attached VF without the need to setup a bond/team between
>>>>> a
>>>>> VF and virtio net device in the guest.
>>>>>
>>>>> The hypervisor needs to unplug the VF device from the guest on the
>>>>> source
>>>>> host and reset the MAC filter of the VF to initiate failover of datapath
>>>>> to
>>>>> virtio before starting the migration. After the migration is completed,
>>>>> the
>>>>> destination hypervisor sets the MAC filter on the VF and plugs it back
>>>>> to
>>>>> the guest to switch over to VF datapath.
>>>>>
>>>>> It is based on netvsc implementation and it may be possible to make this
>>>>> code
>>>>> generic and move it to a common location that can be shared by netvsc
>>>>> and virtio.
>>>>>
>>>>> This patch series is based on the discussion initiated by Jesse on this
>>>>> thread.
>>>>> https://marc.info/?l=linux-virtualization&m=151189725224231&w=2
>>>> How does the notion of a device which is both a bond and a leg of a
>>>> bond fit with Alex's recent discussions about feature propagation?
>>>> Which propagation rules will apply to VirtIO master? Meaning of the
>>>> flags on a software upper device may be different. Why muddy the
>>>> architecture like this and not introduce a synthetic bond device?
>>> It doesn't really fit with the notion I had. I think there may have
>>> been a bit of a disconnect as I have been out for the last week or so
>>> for the holidays.
>>>
>>> My thought on this was that the feature bit should be spawning a new
>>> para-virtual bond device and that bond should have the virto and the
>>> VF as slaves. Also I thought there was some discussion about trying to
>>> reuse as much of the netvsc code as possible for this so that we could
>>> avoid duplication of effort and have the two drivers use the same
>>> approach. It seems like it should be pretty straight forward since you
>>> would have the feature bit in the case of virto, and netvsc just does
>>> this sort of thing by default if I am not mistaken.
>> This patch is mostly based on netvsc implementation. The only change is
>> avoiding the
>> explicit dev_open() call of the VF netdev after a delay. I am assuming that
>> the guest userspace
>> will bring up the VF netdev and the hypervisor will update the MAC filters
>> to switch to
>> the right data path.
>> We could commonize the code and make it shared between netvsc and virtio. Do
>> we want
>> to do this right away or later? If so, what would be a good location for
>> these shared functions?
>> Is it net/core/dev.c?
> No, I would think about starting a new driver file in "/drivers/net/".
> The idea is this driver would be utilized to create a bond
> automatically and set the appropriate registration hooks. If nothing
> else you could probably just call it something generic like virt-bond
> or vbond or whatever.
We are trying to avoid creating another driver or a device. Can we look
into
consolidation of the 2 implementations(virtio & netvsc) as a later patch?
>
>> Also, if we want to go with a solution that creates a bond device, do we
>> want virtio_net/netvsc
>> drivers to create a upper device? Such a solution is already possible via
>> config scripts that can
>> create a bond with virtio and a VF net device as slaves. netvsc and this
>> patch series is trying to
>> make it as simple as possible for the VM to use directly attached devices
>> and support live migration
>> by switching to virtio datapath as a backup during the migration process
>> when the VF device
>> is unplugged.
> We all understand that. But you are making the solution very virtio
> specific. We want to see this be usable for other interfaces such as
> netsc and whatever other virtual interfaces are floating around out
> there.
>
> Also I haven't seen us address what happens as far as how we will
> handle this on the host. My thought was we should have a paired
> interface. Something like veth, but made up of a bond on each end. So
> in the host we should have one bond that has a tap/vhost interface and
> a VF port representor, and on the other we would be looking at the
> virtio interface and the VF. Attaching the tap/vhost to the bond could
> be a way of triggering the feature bit to be set in the virtio. That
> way communication between the guest and the host won't get too
> confusing as you will see all traffic from the bonded MAC address
> always show up on the host side bond instead of potentially showing up
> on two unrelated interfaces. It would also make for a good way to
> resolve the east/west traffic problem on hosts since you could just
> send the broadcast/multicast traffic via the tap/vhost/virtio channel
> instead of having to send it back through the port representor and eat
> up all that PCIe bus traffic.
From the host point of view, here is a simple script that needs to be
run to do the
live migration. We don't need any bond configuration on the host.
virsh detach-interface $DOMAIN hostdev --mac $MAC
ip link set $PF vf $VF_NUM mac $ZERO_MAC
virsh migrate --live $DOMAIN qemu+ssh://$REMOTE_HOST/system
ssh $REMOTE_HOST ip link set $PF vf $VF_NUM mac $MAC
ssh $REMOTE_HOST virsh attach-interface $DOMAIN hostdev $REMOTE_HOSTDEV
--mac $MAC
^ 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