* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Jesper Dangaard Brouer @ 2016-05-02 16:00 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Dave Taht, Jonathan Morton, brouer
In-Reply-To: <1462199668.5535.239.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 02 May 2016 07:34:28 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2016-05-02 at 09:49 +0200, Jesper Dangaard Brouer wrote:
>
> > What about using bulk free of SKBs here?
> >
> > There is a very high probability that we are hitting SLUB slowpath,
> > which involves an expensive locked cmpxchg_double per packet. Instead
> > we can amortize this cost via kmem_cache_free_bulk().
> >
> > Maybe extend kfree_skb_list() to hide the slab/kmem_cache call?
>
> Sounds tricky, because of skb destructors. skb are complex objects.
>
> For each skb, need to free the frags, skb->head, and skb.
It is not that complicated, inside kfree_skb_list(), we just call
skb_release_all(skb) on each SKB first, and then bulk free the SKB's
themselves in the end. Example see, _kfree_skb_defer().
The question is where to store the SKB array needed by kmem_cache_free_bulk.
The easy option is just to use the stack of kfree_skb_list(), but we
have to be careful about the stack size, it might not be so good
because skb_release_all() can be deep and via skb_release_data() invoke
kfree_skb_list() a second time.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next] fq_codel: add batch ability to fq_codel_drop()
From: Eric Dumazet @ 2016-05-02 16:12 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: David Miller, netdev, Dave Taht, Jonathan Morton
In-Reply-To: <20160502180036.25bebdfe@redhat.com>
On Mon, 2016-05-02 at 18:00 +0200, Jesper Dangaard Brouer wrote:
> It is not that complicated, inside kfree_skb_list(), we just call
> skb_release_all(skb) on each SKB first, and then bulk free the SKB's
> themselves in the end. Example see, _kfree_skb_defer().
>
> The question is where to store the SKB array needed by kmem_cache_free_bulk.
>
> The easy option is just to use the stack of kfree_skb_list(), but we
> have to be careful about the stack size, it might not be so good
> because skb_release_all() can be deep and via skb_release_data() invoke
> kfree_skb_list() a second time.
>
It sounds you are reinventing the wheel ;)
If drivers use napi_consume_skb(), qdisc should be able to use it the
same, since BH are disabled in their ->enqueue()/->dequeue() handlers.
This would be a separate patch of course.
This fq_codel fix might need to be backported.
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Jiri Pirko @ 2016-05-02 16:16 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S . Miller, netdev, Eric Dumazet, eladr, idosch
In-Reply-To: <1461605974-4242-2-git-send-email-edumazet@google.com>
Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote:
>SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async()
>so that a SIGIO signal is sent when needed.
>
>tcp_sendmsg() clears the bit.
>tcp_poll() sets the bit when stream is not writeable.
>
>We can avoid two atomic operations by first checking if socket
>is actually interested in the FASYNC business (most sockets in
>real applications do not use AIO, but select()/poll()/epoll())
>
>This also removes one cache line miss to access sk->sk_wq->flags
>in tcp_sendmsg()
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
I just bisected down to this. This is causing a regression for me when
my nfs mount becomes stuck. I can easily reproduce this if you need to
test the fix.
Thanks.
^ permalink raw reply
* Re: [PATCH net 1/2] RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock
From: Santosh Shilimkar @ 2016-05-02 16:20 UTC (permalink / raw)
To: Sowmini Varadhan, netdev, rds-devel; +Cc: davem
In-Reply-To: <470ac585d014a6d8ea1600b8897bdc313e7c2431.1462127059.git.sowmini.varadhan@oracle.com>
On 5/1/2016 4:10 PM, Sowmini Varadhan wrote:
> There is a race condition between rds_send_xmit -> rds_tcp_xmit
> and the code that deals with resolution of duelling syns added
> by commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()").
>
> Specifically, we may end up derefencing a null pointer in rds_send_xmit
> if we have the interleaving sequencee:
> rds_tcp_accept_one rds_send_xmit
>
> conn is RDS_CONN_UP, so
> invoke rds_tcp_xmit
>
> tc = conn->c_transport_data
> rds_tcp_restore_callbacks
> /* reset t_sock */
> null ptr deref from tc->t_sock
>
> The race condition can be avoided without adding the overhead of
> additional locking in the xmit path: have rds_tcp_accept_one wait
> for rds_tcp_xmit threads to complete before resetting callbacks.
> The synchronization can be done in the same manner as rds_conn_shutdown().
> First set the rds_conn_state to something other than RDS_CONN_UP
> (so that new threads cannot get into rds_tcp_xmit()), then wait for
> RDS_IN_XMIT to be cleared in the conn->c_flags indicating that any
> threads in rds_tcp_xmit are done.
>
> Fixes: 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()")
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
Mostly looks correct. A question below.
> net/rds/tcp.c | 2 +-
> net/rds/tcp_listen.c | 40 ++++++++++++++++++++++++----------------
> 2 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/net/rds/tcp.c b/net/rds/tcp.c
> index 61ed2a8..9134544 100644
> --- a/net/rds/tcp.c
> +++ b/net/rds/tcp.c
> @@ -127,7 +127,7 @@ void rds_tcp_restore_callbacks(struct socket *sock,
>
> /*
> * This is the only path that sets tc->t_sock. Send and receive trust that
> - * it is set. The RDS_CONN_CONNECTED bit protects those paths from being
> + * it is set. The RDS_CONN_UP bit protects those paths from being
> * called while it isn't set.
> */
> void rds_tcp_set_callbacks(struct socket *sock, struct rds_connection *conn)
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index 0936a4a..0896187 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -115,24 +115,32 @@ int rds_tcp_accept_one(struct socket *sock)
> * rds_tcp_state_change() will do that cleanup
> */
> rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
> - if (rs_tcp->t_sock &&
> - ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
> - struct sock *nsk = new_sock->sk;
> -
> - nsk->sk_user_data = NULL;
> - nsk->sk_prot->disconnect(nsk, 0);
> - tcp_done(nsk);
> - new_sock = NULL;
> - ret = 0;
> - goto out;
> - } else if (rs_tcp->t_sock) {
> - rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
> - conn->c_outgoing = 0;
> - }
> -
> rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
> + if (rs_tcp->t_sock) {
> + /* Need to resolve a duelling SYN between peers.
> + * We have an outstanding SYN to this peer, which may
> + * potentially have transitioned to the RDS_CONN_UP state,
> + * so we must quiesce any send threads before resetting
> + * c_transport_data.
> + */
> + wait_event(conn->c_waitq,
> + !test_bit(RDS_IN_XMIT, &conn->c_flags));
Would it be good to check the return value of rds_conn_transition()
since if CONN is already UP above will fail and then send message
might again race and we will let message through even though passive
hasn't finished its connection.
> + if (ntohl(inet->inet_saddr) < ntohl(inet->inet_daddr)) {
> + struct sock *nsk = new_sock->sk;
> +
> + nsk->sk_user_data = NULL;
> + nsk->sk_prot->disconnect(nsk, 0);
> + tcp_done(nsk);
> + new_sock = NULL;
> + ret = 0;
> + goto out;
> + } else if (rs_tcp->t_sock) {
> + rds_tcp_restore_callbacks(rs_tcp->t_sock, rs_tcp);
> + conn->c_outgoing = 0;
> + }
> + }
> rds_tcp_set_callbacks(new_sock, conn);
> - rds_connect_complete(conn);
> + rds_connect_complete(conn); /* marks RDS_CONN_UP */
> new_sock = NULL;
> ret = 0;
>
>
^ permalink raw reply
* [PATCHv4] netem: Segment GSO packets on enqueue
From: Neil Horman @ 2016-05-02 16:20 UTC (permalink / raw)
To: netdev
Cc: Neil Horman, Jamal Hadi Salim, David S. Miller, netem,
eric.dumazet, stephen
In-Reply-To: <1461692618-21333-1-git-send-email-nhorman@tuxdriver.com>
This was recently reported to me, and reproduced on the latest net kernel,
when attempting to run netperf from a host that had a netem qdisc attached
to the egress interface:
[ 788.073771] ---------------------[ cut here ]---------------------------
[ 788.096716] WARNING: at net/core/dev.c:2253 skb_warn_bad_offload+0xcd/0xda()
[ 788.129521] bnx2: caps=(0x00000001801949b3, 0x0000000000000000) len=2962
data_len=0 gso_size=1448 gso_type=1 ip_summed=3
[ 788.182150] Modules linked in: sch_netem kvm_amd kvm crc32_pclmul ipmi_ssif
ghash_clmulni_intel sp5100_tco amd64_edac_mod aesni_intel lrw gf128mul
glue_helper ablk_helper edac_mce_amd cryptd pcspkr sg edac_core hpilo ipmi_si
i2c_piix4 k10temp fam15h_power hpwdt ipmi_msghandler shpchp acpi_power_meter
pcc_cpufreq nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables xfs libcrc32c
sd_mod crc_t10dif crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt
i2c_algo_bit drm_kms_helper ahci ata_generic pata_acpi ttm libahci
crct10dif_pclmul pata_atiixp tg3 libata crct10dif_common drm crc32c_intel ptp
serio_raw bnx2 r8169 hpsa pps_core i2c_core mii dm_mirror dm_region_hash dm_log
dm_mod
[ 788.465294] CPU: 16 PID: 0 Comm: swapper/16 Tainted: G W
------------ 3.10.0-327.el7.x86_64 #1
[ 788.511521] Hardware name: HP ProLiant DL385p Gen8, BIOS A28 12/17/2012
[ 788.542260] ffff880437c036b8 f7afc56532a53db9 ffff880437c03670
ffffffff816351f1
[ 788.576332] ffff880437c036a8 ffffffff8107b200 ffff880633e74200
ffff880231674000
[ 788.611943] 0000000000000001 0000000000000003 0000000000000000
ffff880437c03710
[ 788.647241] Call Trace:
[ 788.658817] <IRQ> [<ffffffff816351f1>] dump_stack+0x19/0x1b
[ 788.686193] [<ffffffff8107b200>] warn_slowpath_common+0x70/0xb0
[ 788.713803] [<ffffffff8107b29c>] warn_slowpath_fmt+0x5c/0x80
[ 788.741314] [<ffffffff812f92f3>] ? ___ratelimit+0x93/0x100
[ 788.767018] [<ffffffff81637f49>] skb_warn_bad_offload+0xcd/0xda
[ 788.796117] [<ffffffff8152950c>] skb_checksum_help+0x17c/0x190
[ 788.823392] [<ffffffffa01463a1>] netem_enqueue+0x741/0x7c0 [sch_netem]
[ 788.854487] [<ffffffff8152cb58>] dev_queue_xmit+0x2a8/0x570
[ 788.880870] [<ffffffff8156ae1d>] ip_finish_output+0x53d/0x7d0
...
The problem occurs because netem is not prepared to handle GSO packets (as it
uses skb_checksum_help in its enqueue path, which cannot manipulate these
frames).
The solution I think is to simply segment the skb in a simmilar fashion to the
way we do in __dev_queue_xmit (via validate_xmit_skb), with some minor changes.
When we decide to corrupt an skb, if the frame is GSO, we segment it, corrupt
the first segment, and enqueue the remaining ones.
tested successfully by myself on the latest net kernel, to which this applies
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jamal Hadi Salim <jhs@mojatatu.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: netem@lists.linux-foundation.org
CC: eric.dumazet@gmail.com
CC: stephen@networkplumber.org
---
Change Notes:
V2) As per request from Eric Dumazet, I rewrote this to limit the need to
segment the skb. Instead of doing so unilaterally, we no only do so now when the
netem qdisc requires determines that a packet must be corrupted, thus avoiding
the failure in skb_checksum_help. This still leaves open concerns with
statistical measurements made on GSO packets being dropped or reordered (i.e.
they are counted as a single packet rather than multiple packets), but I'd
rather fix the immediate problem before we go rewriting everything to fix that
larger issue.
V3) Added back missing call to qdisc_tree_reduce_backlog that I misplaced in the
V2 change.
V4) Fix up length computation and return code. Also clean up some patch
formatting
---
net/sched/sch_netem.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 59 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 9640bb3..4befe97 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -395,6 +395,25 @@ static void tfifo_enqueue(struct sk_buff *nskb, struct Qdisc *sch)
sch->q.qlen++;
}
+/* netem can't properly corrupt a megapacket (like we get from GSO), so instead
+ * when we statistically choose to corrupt one, we instead segment it, returning
+ * the first packet to be corrupted, and re-enqueue the remaining frames
+ */
+static struct sk_buff *netem_segment(struct sk_buff *skb, struct Qdisc *sch)
+{
+ struct sk_buff *segs;
+ netdev_features_t features = netif_skb_features(skb);
+
+ segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
+
+ if (IS_ERR_OR_NULL(segs)) {
+ qdisc_reshape_fail(skb, sch);
+ return NULL;
+ }
+ consume_skb(skb);
+ return segs;
+}
+
/*
* Insert one skb into qdisc.
* Note: parent depends on return value to account for queue length.
@@ -407,7 +426,11 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
/* We don't fill cb now as skb_unshare() may invalidate it */
struct netem_skb_cb *cb;
struct sk_buff *skb2;
+ struct sk_buff *segs = NULL;
+ unsigned int len = 0, last_len, prev_len = qdisc_pkt_len(skb);
+ int nb = 0;
int count = 1;
+ int rc = NET_XMIT_SUCCESS;
/* Random duplication */
if (q->duplicate && q->duplicate >= get_crandom(&q->dup_cor))
@@ -453,10 +476,23 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
* do it now in software before we mangle it.
*/
if (q->corrupt && q->corrupt >= get_crandom(&q->corrupt_cor)) {
+ if (skb_is_gso(skb)) {
+ segs = netem_segment(skb, sch);
+ if (!segs)
+ return NET_XMIT_DROP;
+ } else {
+ segs = skb;
+ }
+
+ skb = segs;
+ segs = segs->next;
+
if (!(skb = skb_unshare(skb, GFP_ATOMIC)) ||
(skb->ip_summed == CHECKSUM_PARTIAL &&
- skb_checksum_help(skb)))
- return qdisc_drop(skb, sch);
+ skb_checksum_help(skb))) {
+ rc = qdisc_drop(skb, sch);
+ goto finish_segs;
+ }
skb->data[prandom_u32() % skb_headlen(skb)] ^=
1<<(prandom_u32() % 8);
@@ -516,6 +552,27 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch)
sch->qstats.requeues++;
}
+finish_segs:
+ if (segs) {
+ while (segs) {
+ skb2 = segs->next;
+ segs->next = NULL;
+ qdisc_skb_cb(segs)->pkt_len = segs->len;
+ last_len = segs->len;
+ rc = qdisc_enqueue(segs, sch);
+ if (rc != NET_XMIT_SUCCESS) {
+ if (net_xmit_drop_count(rc))
+ qdisc_qstats_drop(sch);
+ } else {
+ nb++;
+ len += last_len;
+ }
+ segs = skb2;
+ }
+ sch->q.qlen += nb;
+ if (nb > 1)
+ qdisc_tree_reduce_backlog(sch, 1 - nb, prev_len - len);
+ }
return NET_XMIT_SUCCESS;
}
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net-next 1/2] net: SOCKWQ_ASYNC_NOSPACE optimizations
From: Eric Dumazet @ 2016-05-02 16:22 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Eric Dumazet, David S . Miller, netdev, eladr, idosch
In-Reply-To: <20160502161602.GA1984@nanopsycho.orion>
On Mon, 2016-05-02 at 18:16 +0200, Jiri Pirko wrote:
> Mon, Apr 25, 2016 at 07:39:32PM CEST, edumazet@google.com wrote:
> >SOCKWQ_ASYNC_NOSPACE is tested in sock_wake_async()
> >so that a SIGIO signal is sent when needed.
> >
> >tcp_sendmsg() clears the bit.
> >tcp_poll() sets the bit when stream is not writeable.
> >
> >We can avoid two atomic operations by first checking if socket
> >is actually interested in the FASYNC business (most sockets in
> >real applications do not use AIO, but select()/poll()/epoll())
> >
> >This also removes one cache line miss to access sk->sk_wq->flags
> >in tcp_sendmsg()
> >
> >Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> I just bisected down to this. This is causing a regression for me when
> my nfs mount becomes stuck. I can easily reproduce this if you need to
> test the fix.
What do you mean by 'when nfs mount becomes stuck' ?
Is this patch making nfs not functional , or does it make recovery from
some nfs error bad ?
Thanks.
^ permalink raw reply
* [net PATCH 0/2] Fixes for tunnel checksum and segmentation offloads
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
This patch series is a subset of patches I had submitted for net-next. I
plan to drop these two patches from the v3 of "Fix Tunnel features and
enable GSO partial for several drivers" and I am instead submitting them
for net since these are truly fixes and likely will need to be backported
to stable branches.
This series addresses 2 specific issues. The first is that we could
request TSO on a v4 inner header while not supporting checksum offload of
the outer IPv6 header. The second is that we could request an IPv6 inner
checksum offload without validating that we could actually support an inner
IPv6 checksum offload.
---
Alexander Duyck (2):
net: Disable segmentation if checksumming is not supported
vxlan: Add checksum check to the features check function
include/linux/if_ether.h | 5 +++++
include/net/vxlan.h | 4 +++-
net/core/dev.c | 2 +-
3 files changed, 9 insertions(+), 2 deletions(-)
^ permalink raw reply
* [net PATCH 1/2] net: Disable segmentation if checksumming is not supported
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
In-Reply-To: <20160502161621.11701.9271.stgit@ahduyck-xeon-server>
In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
offload for tunnels. With this being the case we should disable GSO in
addition to the checksum offload features when we find that a device cannot
perform a checksum on a given packet type.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 77a71cd68535..5c925ac50b95 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2802,7 +2802,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
if (skb->ip_summed != CHECKSUM_NONE &&
!can_checksum_protocol(features, type)) {
- features &= ~NETIF_F_CSUM_MASK;
+ features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
} else if (illegal_highdma(skb->dev, skb)) {
features &= ~NETIF_F_SG;
}
^ permalink raw reply related
* [net PATCH 2/2] vxlan: Add checksum check to the features check function
From: Alexander Duyck @ 2016-05-02 16:25 UTC (permalink / raw)
To: netdev, ogerlitz, davem, alexander.duyck
In-Reply-To: <20160502161621.11701.9271.stgit@ahduyck-xeon-server>
We need to perform an additional check on the inner headers to determine if
we can offload the checksum for them. Previously this check didn't occur
so we would generate an invalid frame in the case of an IPv6 header
encapsulated inside of an IPv4 tunnel. To fix this I added a secondary
check to vxlan_features_check so that we can verify that we can offload the
inner checksum.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
include/linux/if_ether.h | 5 +++++
include/net/vxlan.h | 4 +++-
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/linux/if_ether.h b/include/linux/if_ether.h
index d5569734f672..548fd535fd02 100644
--- a/include/linux/if_ether.h
+++ b/include/linux/if_ether.h
@@ -28,6 +28,11 @@ static inline struct ethhdr *eth_hdr(const struct sk_buff *skb)
return (struct ethhdr *)skb_mac_header(skb);
}
+static inline struct ethhdr *inner_eth_hdr(const struct sk_buff *skb)
+{
+ return (struct ethhdr *)skb_inner_mac_header(skb);
+}
+
int eth_header_parse(const struct sk_buff *skb, unsigned char *haddr);
extern ssize_t sysfs_format_mac(char *buf, const unsigned char *addr, int len);
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 73ed2e951c02..35437c779da8 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -252,7 +252,9 @@ static inline netdev_features_t vxlan_features_check(struct sk_buff *skb,
(skb->inner_protocol_type != ENCAP_TYPE_ETHER ||
skb->inner_protocol != htons(ETH_P_TEB) ||
(skb_inner_mac_header(skb) - skb_transport_header(skb) !=
- sizeof(struct udphdr) + sizeof(struct vxlanhdr))))
+ sizeof(struct udphdr) + sizeof(struct vxlanhdr)) ||
+ (skb->ip_summed != CHECKSUM_NONE &&
+ !can_checksum_protocol(features, inner_eth_hdr(skb)->h_proto))))
return features & ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
return features;
^ permalink raw reply related
* Re: [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect() paths on t_conn_lock.
From: Santosh Shilimkar @ 2016-05-02 16:33 UTC (permalink / raw)
To: Sowmini Varadhan, netdev, rds-devel; +Cc: davem
In-Reply-To: <5fc507116182afdda4b824173008e09d5d464a33.1462127059.git.sowmini.varadhan@oracle.com>
On 5/1/2016 4:10 PM, Sowmini Varadhan wrote:
> An arbitration scheme for duelling SYNs is implemented as part of
> commit 241b271952eb ("RDS-TCP: Reset tcp callbacks if re-using an
> outgoing socket in rds_tcp_accept_one()") which ensures that both nodes
> involved will arrive at the same arbitration decision. However, this
> needs to be synchronized with an outgoing SYN to be generated by
> rds_tcp_conn_connect(). This commit achieves the synchronization
> through the t_conn_lock mutex in struct rds_tcp_connection.
>
> The rds_conn_state is checked in rds_tcp_conn_connect() after acquiring
> the t_conn_lock mutex. A SYN is sent out only if the RDS connection is
> not already UP (an UP would indicate that rds_tcp_accept_one() has
> completed 3WH, so no SYN needs to be generated).
>
> Similarly, the rds_conn_state is checked in rds_tcp_accept_one() after
> acquiring the t_conn_lock mutex. The only acceptable states (to
> allow continuation of the arbitration logic) are UP (i.e., outgoing SYN
> was SYN-ACKed by peer after it sent us the SYN) or CONNECTING (we sent
> outgoing SYN before we saw incoming SYN).
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> net/rds/tcp.c | 1 +
> net/rds/tcp.h | 4 ++++
> net/rds/tcp_connect.c | 8 ++++++++
> net/rds/tcp_listen.c | 30 ++++++++++++++++++++----------
> 4 files changed, 33 insertions(+), 10 deletions(-)
>
[...]
> diff --git a/net/rds/tcp_connect.c b/net/rds/tcp_connect.c
> index 5cb1687..49a3fcf 100644
> --- a/net/rds/tcp_connect.c
> +++ b/net/rds/tcp_connect.c
> @@ -78,7 +78,14 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
> struct socket *sock = NULL;
> struct sockaddr_in src, dest;
> int ret;
> + struct rds_tcp_connection *tc = conn->c_transport_data;
> +
> + mutex_lock(&tc->t_conn_lock);
>
> + if (rds_conn_up(conn)) {
> + mutex_unlock(&tc->t_conn_lock);
> + return 0;
> + }
> ret = sock_create_kern(rds_conn_net(conn), PF_INET,
> SOCK_STREAM, IPPROTO_TCP, &sock);
> if (ret < 0)
> @@ -120,6 +127,7 @@ int rds_tcp_conn_connect(struct rds_connection *conn)
> }
>
> out:
> + mutex_unlock(&tc->t_conn_lock);
Just wondering whether the spin_lock() would better here considering
entry into rds_tcp_conn_connect() & rds_tcp_accept_one() might be
from softirq context. Ignore it if its not applicable.
> if (sock)
> sock_release(sock);
> return ret;
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index 0896187..cc8496f 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -76,7 +76,9 @@ int rds_tcp_accept_one(struct socket *sock)
> struct rds_connection *conn;
> int ret;
> struct inet_sock *inet;
> - struct rds_tcp_connection *rs_tcp;
> + struct rds_tcp_connection *rs_tcp = NULL;
> + int conn_state;
> + struct sock *nsk;
>
> ret = sock_create_kern(sock_net(sock->sk), sock->sk->sk_family,
> sock->sk->sk_type, sock->sk->sk_protocol,
> @@ -116,6 +118,10 @@ int rds_tcp_accept_one(struct socket *sock)
> */
> rs_tcp = (struct rds_tcp_connection *)conn->c_transport_data;
> rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
Like patch 1/2, probably we can leverage return value of above.
> + conn_state = rds_conn_state(conn);
> + if (conn_state != RDS_CONN_CONNECTING && conn_state != RDS_CONN_UP)
You probably don't need the local 'conn_state' and below should work.
if (!rds_conn_connecting(conn) && !rds_conn_up(conn))
Regards,
Santosh
^ permalink raw reply
* Re: [net PATCH 1/2] net: Disable segmentation if checksumming is not supported
From: Tom Herbert @ 2016-05-02 16:33 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Or Gerlitz, David S. Miller,
Alexander Duyck
In-Reply-To: <20160502162510.11701.82922.stgit@ahduyck-xeon-server>
On Mon, May 2, 2016 at 9:25 AM, Alexander Duyck <aduyck@mirantis.com> wrote:
> In the case of the mlx4 and mlx5 driver they do not support IPv6 checksum
> offload for tunnels. With this being the case we should disable GSO in
> addition to the checksum offload features when we find that a device cannot
> perform a checksum on a given packet type.
>
I'm not sure I understand this. If device can't support checksum
offload for tunnels doesn't that mean we have to do the checksum on
host regardless of whether GSO is being done?
Tom
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> net/core/dev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 77a71cd68535..5c925ac50b95 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2802,7 +2802,7 @@ static netdev_features_t harmonize_features(struct sk_buff *skb,
>
> if (skb->ip_summed != CHECKSUM_NONE &&
> !can_checksum_protocol(features, type)) {
> - features &= ~NETIF_F_CSUM_MASK;
> + features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
> } else if (illegal_highdma(skb->dev, skb)) {
> features &= ~NETIF_F_SG;
> }
>
^ permalink raw reply
* Re: [PATCH net 1/2] RDS:TCP: Synchronize rds_tcp_accept_one with rds_send_xmit when resetting t_sock
From: Sowmini Varadhan @ 2016-05-02 16:37 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: netdev, rds-devel, davem
In-Reply-To: <7fac68dc-0ff5-36a5-6a3d-df802d8db82d@oracle.com>
On (05/02/16 09:20), Santosh Shilimkar wrote:
> > rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
> >+ if (rs_tcp->t_sock) {
> >+ /* Need to resolve a duelling SYN between peers.
> >+ * We have an outstanding SYN to this peer, which may
> >+ * potentially have transitioned to the RDS_CONN_UP state,
> >+ * so we must quiesce any send threads before resetting
> >+ * c_transport_data.
> >+ */
> >+ wait_event(conn->c_waitq,
> >+ !test_bit(RDS_IN_XMIT, &conn->c_flags));
> Would it be good to check the return value of rds_conn_transition()
> since if CONN is already UP above will fail and then send message
> might again race and we will let message through even though passive
> hasn't finished its connection.
no, that was the original issue that I was running into, which needed
commit 241b2719 - prior to that commit, if the conn was already UP,
we'd end up doing a rds_conn_drop on a good connection, and both sides
would end up in a pair of infinite 3WH loops. Even if we dont do
a rds_conn_drop on the UP connection, we've just (before
rds_tcp_accept_one) sent out a syn-ack on the incoming syn, and now
need to RST that syn-ac. The other side is going to receive the rst,
and get confused about what to clean up (since there's already an UP
connection going on).
In short, when there is a duel, it's cleanest to have a deterministic
arbitration- both sides use the numeric value of saddr and faddr to
figure out which side is active, which side is passive. (Thus the
basis on the BGP router-id based model for 241b2719)
FWIW, much of this is actually a corner case- in practice, its not
frequent to have syns crossing each other at "almost the same time".
--Sowmini
^ permalink raw reply
* [net-next PATCH v3 0/8] Fix Tunnel features and enable GSO partial for several drivers
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
This patch series is meant to allow us to get the best performance possible
for Mellanox ConnectX-3/4 and Broadcom NetXtreme-C/E adapters in terms of
VXLAN and GRE tunnels.
The first 3 patches address issues I found in regards to GSO_PARTIAL and
TSO_MANGLEID.
The next 4 patches go through and enable GSO_PARTIAL for VXLAN tunnels that
have an outer checksum enabled, and then enable IPv6 support where I can.
One outstanding issue is that I wasn't able to get offloads working with
outer IPv6 headers on mlx4. However that wasn't a feature that was enabled
before so it isn't technically a regression, however I believe Engineers
from Mellanox said they would look into it since they thought it should be
supported.
The last patch enables GSO_PARTIAL for VXLAN and GRE tunnels on the bnxt
driver. One piece of feedback I received on the patch was that the
hardware has globally set IPv6 UDP tunnels to always have the checksum
field computed. I plan to work with Broadcom to get that addressed so that
we only populate the checksum field if it was requested by the network
stack.
v2: Rebased patches off of latest changes to the mlx4/mlx5 drivers.
Added bnxt driver patch as I received feedback on the RFC.
v3: Moved 2 patches into series for net as they were generic fixes.
Added patch to disable GSO partial if frame is less than 2x size of MSS
There are outstanding issues as called out above that need to be
addressed, however they were present before these patches so it isn't
as if they introduce a regression. In addition gains can be easily
seen so there should be no issue with applying the driver patches while
the IPv6 mlx4_en and bnxt issues are being researched.
---
Alexander Duyck (8):
gso: Do not perform partial GSO if number of partial segments is 1 or less
gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
net/mlx4_en: Add support for UDP tunnel segmentation with outer checksum offload
net/mlx4_en: Add support for inner IPv6 checksum offloads and TSO
net/mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
net/mlx5e: Fix IPv6 tunnel checksum offload
bnxt: Add support for segmentation of tunnels with outer checksums
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 ++++-
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 38 +++++++++++++++++----
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 15 +++++++-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 10 ++++--
net/core/dev.c | 4 ++
net/core/skbuff.c | 11 ++++--
6 files changed, 69 insertions(+), 18 deletions(-)
^ permalink raw reply
* [net-next PATCH v3 1/8] gso: Do not perform partial GSO if number of partial segments is 1 or less
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
In the event that the number of partial segments is equal to 1 we don't
really need to perform partial segmentation offload. As such we should
skip multiplying the MSS and instead just clear the partial_segs value
since it will not provide any gain to advertise the frame as being GSO when
it is a single frame.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 7a1d48983f81..b8dd2d2e2256 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3101,7 +3101,10 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
*/
if (features & NETIF_F_GSO_PARTIAL) {
partial_segs = len / mss;
- mss *= partial_segs;
+ if (partial_segs > 1)
+ mss *= partial_segs;
+ else
+ partial_segs = 0;
}
headroom = skb_headroom(head_skb);
^ permalink raw reply related
* [net-next PATCH v3 2/8] gso: Only allow GSO_PARTIAL if we can checksum the inner protocol
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
This patch addresses a possible issue that can occur if we get into any odd
corner cases where we support TSO for a given protocol but not the checksum
or scatter-gather offload. There are few drivers floating around that
setup their tunnels this way and by enforcing the checksum piece we can
avoid mangling any frames.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b8dd2d2e2256..5586be93632f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3080,8 +3080,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
unsigned int headroom;
unsigned int len = head_skb->len;
__be16 proto;
- bool csum;
- int sg = !!(features & NETIF_F_SG);
+ bool csum, sg;
int nfrags = skb_shinfo(head_skb)->nr_frags;
int err = -ENOMEM;
int i = 0;
@@ -3093,13 +3092,14 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
if (unlikely(!proto))
return ERR_PTR(-EINVAL);
+ sg = !!(features & NETIF_F_SG);
csum = !!can_checksum_protocol(features, proto);
/* GSO partial only requires that we trim off any excess that
* doesn't fit into an MSS sized block, so take care of that
* now.
*/
- if (features & NETIF_F_GSO_PARTIAL) {
+ if (sg && csum && (features & NETIF_F_GSO_PARTIAL)) {
partial_segs = len / mss;
if (partial_segs > 1)
mss *= partial_segs;
^ permalink raw reply related
* [net-next PATCH v3 3/8] net: Fix netdev_fix_features so that TSO_MANGLEID is only available with TSO
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
This change makes it so that we will strip the TSO_MANGLEID bit if TSO is
not present. This way we will also handle ECN correctly of TSO is not
present.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/dev.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/core/dev.c b/net/core/dev.c
index 673d1f118bfb..e98ba63fe280 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6721,6 +6721,10 @@ static netdev_features_t netdev_fix_features(struct net_device *dev,
features &= ~NETIF_F_TSO6;
}
+ /* TSO with IPv4 ID mangling requires IPv4 TSO be enabled */
+ if ((features & NETIF_F_TSO_MANGLEID) && !(features & NETIF_F_TSO))
+ features &= ~NETIF_F_TSO_MANGLEID;
+
/* TSO ECN requires that TSO is present as well. */
if ((features & NETIF_F_ALL_TSO) == NETIF_F_TSO_ECN)
features &= ~NETIF_F_TSO_ECN;
^ permalink raw reply related
* [net-next PATCH v3 4/8] net/mlx4_en: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
This patch assumes that the mlx4 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index 8bd143dda95d..bce37cbfde24 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2358,7 +2358,9 @@ out:
/* set offloads */
priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL;
+ NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
}
static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
@@ -2368,7 +2370,9 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
vxlan_del_task);
/* unset offloads */
priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL);
+ NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL);
ret = mlx4_SET_PORT_VXLAN(priv->mdev->dev, priv->port,
VXLAN_STEER_BY_OUTER_MAC, 0);
@@ -2992,8 +2996,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
}
if (mdev->dev->caps.tunnel_offload_mode == MLX4_TUNNEL_OFFLOAD_MODE_VXLAN) {
- dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
- dev->features |= NETIF_F_GSO_UDP_TUNNEL;
+ dev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ dev->features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
}
mdev->pndev[port] = dev;
^ permalink raw reply related
* [PATCH v2] rtlwifi: Fix logic error in enter/exit power-save mode
From: Wang YanQing @ 2016-05-02 16:38 UTC (permalink / raw)
To: Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ
Cc: kvalo-sgV2jX0FEOL9JmXXK+q4OQ, chaoming_li-kXabqFNEczNtrwSWzY7KCg,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In commit a269913c52ad ("rtlwifi: Rework rtl_lps_leave() and
rtl_lps_enter() to use work queue"), the tests for enter/exit
power-save mode were inverted. With this change applied, the
wifi connection becomes much more stable.
Fixes: a269913c52ad ("rtlwifi: Rework rtl_lps_leave() and rtl_lps_enter() to use work queue")
Signed-off-by: Wang YanQing <udknight-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
CC: Stable <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> [3.10+]
---
Hi, Larry!
Because commit a269913c52ad is the first commit bring this problem,
so maybe use above commit message is ok, right? And stable kernels
3.10-3.18 don't have commit fd09ff958777, but have a269913c52ad.
Thanks for suggestion concerning to good subject and commit message
writing, it is harder than coding sometimes:)
Changes:
v1-v2:
1: Fix subject and commit message.
drivers/net/wireless/realtek/rtlwifi/base.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index c74eb13..264466f 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -1660,9 +1660,9 @@ void rtl_watchdog_wq_callback(void *data)
if (((rtlpriv->link_info.num_rx_inperiod +
rtlpriv->link_info.num_tx_inperiod) > 8) ||
(rtlpriv->link_info.num_rx_inperiod > 2))
- rtl_lps_enter(hw);
- else
rtl_lps_leave(hw);
+ else
+ rtl_lps_enter(hw);
}
rtlpriv->link_info.num_rx_inperiod = 0;
--
1.8.5.6.2.g3d8a54e.dirty
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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
* [net-next PATCH v3 5/8] net/mlx4_en: Add support for inner IPv6 checksum offloads and TSO
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
>From what I can tell the ConnectX-3 will support an inner IPv6 checksum and
segmentation offload, however it cannot support outer IPv6 headers. This
assumption is based on the fact that I could see the checksum being
offloaded for inner header on IPv4 tunnels, but not on IPv6 tunnels.
For this reason I am adding the feature to the hw_enc_features and adding
an extra check to the features_check call that will disable GSO and
checksum offload in the case that the encapsulated frame has an outer IP
version of that is not 4. The check in mlx4_en_features_check could be
removed if at some point in the future a fix is found that allows the
hardware to offload segmentation/checksum on tunnels with an outer IPv6
header.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 25 +++++++++++++++++++-----
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 15 ++++++++++++--
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index bce37cbfde24..6f28ac58251c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -2357,8 +2357,10 @@ out:
}
/* set offloads */
- priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ priv->dev->hw_enc_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM |
+ NETIF_F_TSO | NETIF_F_TSO6 |
+ NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
}
@@ -2369,8 +2371,10 @@ static void mlx4_en_del_vxlan_offloads(struct work_struct *work)
struct mlx4_en_priv *priv = container_of(work, struct mlx4_en_priv,
vxlan_del_task);
/* unset offloads */
- priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
- NETIF_F_TSO | NETIF_F_GSO_UDP_TUNNEL |
+ priv->dev->hw_enc_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
+ NETIF_F_RXCSUM |
+ NETIF_F_TSO | NETIF_F_TSO6 |
+ NETIF_F_GSO_UDP_TUNNEL |
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL);
@@ -2431,7 +2435,18 @@ static netdev_features_t mlx4_en_features_check(struct sk_buff *skb,
netdev_features_t features)
{
features = vlan_features_check(skb, features);
- return vxlan_features_check(skb, features);
+ features = vxlan_features_check(skb, features);
+
+ /* The ConnectX-3 doesn't support outer IPv6 checksums but it does
+ * support inner IPv6 checksums and segmentation so we need to
+ * strip that feature if this is an IPv6 encapsulated frame.
+ */
+ if (skb->encapsulation &&
+ (skb->ip_summed == CHECKSUM_PARTIAL) &&
+ (ip_hdr(skb)->version != 4))
+ features &= ~(NETIF_F_CSUM_MASK | NETIF_F_GSO_MASK);
+
+ return features;
}
#endif
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index a386f047c1af..0f206a95429c 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -41,6 +41,7 @@
#include <linux/vmalloc.h>
#include <linux/tcp.h>
#include <linux/ip.h>
+#include <linux/ipv6.h>
#include <linux/moduleparam.h>
#include "mlx4_en.h"
@@ -920,8 +921,18 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
tx_ind, fragptr);
if (skb->encapsulation) {
- struct iphdr *ipv4 = (struct iphdr *)skb_inner_network_header(skb);
- if (ipv4->protocol == IPPROTO_TCP || ipv4->protocol == IPPROTO_UDP)
+ union {
+ struct iphdr *v4;
+ struct ipv6hdr *v6;
+ unsigned char *hdr;
+ } ip;
+ u8 proto;
+
+ ip.hdr = skb_inner_network_header(skb);
+ proto = (ip.v4->version == 4) ? ip.v4->protocol :
+ ip.v6->nexthdr;
+
+ if (proto == IPPROTO_TCP || proto == IPPROTO_UDP)
op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP | MLX4_WQE_CTRL_ILP);
else
op_own |= cpu_to_be32(MLX4_WQE_CTRL_IIP);
^ permalink raw reply related
* [net-next PATCH v3 6/8] net/mlx5e: Add support for UDP tunnel segmentation with outer checksum offload
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
This patch assumes that the mlx5 hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP headers.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 4ccfc1ac62c5..2d6aaad77d62 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2803,13 +2803,18 @@ static void mlx5e_build_netdev(struct net_device *netdev)
netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
if (mlx5e_vxlan_allowed(mdev)) {
- netdev->hw_features |= NETIF_F_GSO_UDP_TUNNEL;
+ netdev->hw_features |= NETIF_F_GSO_UDP_TUNNEL |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
netdev->hw_enc_features |= NETIF_F_IP_CSUM;
netdev->hw_enc_features |= NETIF_F_RXCSUM;
netdev->hw_enc_features |= NETIF_F_TSO;
netdev->hw_enc_features |= NETIF_F_TSO6;
netdev->hw_enc_features |= NETIF_F_RXHASH;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
+ netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_PARTIAL;
+ netdev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM;
}
mlx5_query_port_fcs(mdev, &fcs_supported, &fcs_enabled);
^ permalink raw reply related
* [net-next PATCH v3 7/8] net/mlx5e: Fix IPv6 tunnel checksum offload
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
The mlx5 driver exposes support for TSO6 but not IPv6 csum for hardware
encapsulated tunnels. This leads to issues as it triggers warnings in
skb_checksum_help as it ends up being called as we report supporting the
segmentation but not the checksumming for IPv6 frames.
This patch corrects that and drops 2 features that don't actually need to
be supported in hw_enc_features since they are Rx features and don't
actually impact anything by being present in hw_enc_features.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 2d6aaad77d62..409916c18b86 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -2807,10 +2807,9 @@ static void mlx5e_build_netdev(struct net_device *netdev)
NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
netdev->hw_enc_features |= NETIF_F_IP_CSUM;
- netdev->hw_enc_features |= NETIF_F_RXCSUM;
+ netdev->hw_enc_features |= NETIF_F_IPV6_CSUM;
netdev->hw_enc_features |= NETIF_F_TSO;
netdev->hw_enc_features |= NETIF_F_TSO6;
- netdev->hw_enc_features |= NETIF_F_RXHASH;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL;
netdev->hw_enc_features |= NETIF_F_GSO_UDP_TUNNEL_CSUM |
NETIF_F_GSO_PARTIAL;
^ permalink raw reply related
* [net-next PATCH v3 8/8] bnxt: Add support for segmentation of tunnels with outer checksums
From: Alexander Duyck @ 2016-05-02 16:38 UTC (permalink / raw)
To: talal, netdev, michael.chan, alexander.duyck, davem, galp,
ogerlitz, eranbe
In-Reply-To: <20160502162828.11809.92444.stgit@ahduyck-xeon-server>
This patch assumes that the bnxt hardware will ignore existing IPv4/v6
header fields for length and checksum as well as the length and checksum
fields for outer UDP and GRE headers.
I have been told by Michael Chan that this is working. Though this might
be somewhat redundant for IPv6 as they are forcing the checksum to be
computed for all IPv6 frames that are offloaded. A follow-up patch may be
necessary in order to fix this as it is essentially mangling the outer IPv6
headers to add a checksum where none was requested.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 4645c44e7c15..ae668476fff0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6194,14 +6194,19 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_TSO | NETIF_F_TSO6 |
NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
- NETIF_F_RXHASH |
+ NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
+ NETIF_F_GSO_PARTIAL | NETIF_F_RXHASH |
NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_GRO;
dev->hw_enc_features =
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM | NETIF_F_SG |
NETIF_F_TSO | NETIF_F_TSO6 |
NETIF_F_GSO_UDP_TUNNEL | NETIF_F_GSO_GRE |
- NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT;
+ NETIF_F_GSO_UDP_TUNNEL_CSUM | NETIF_F_GSO_GRE_CSUM |
+ NETIF_F_GSO_IPIP | NETIF_F_GSO_SIT |
+ NETIF_F_GSO_PARTIAL;
+ dev->gso_partial_features = NETIF_F_GSO_UDP_TUNNEL_CSUM |
+ NETIF_F_GSO_GRE_CSUM;
dev->vlan_features = dev->hw_features | NETIF_F_HIGHDMA;
dev->hw_features |= NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX |
NETIF_F_HW_VLAN_STAG_RX | NETIF_F_HW_VLAN_STAG_TX;
^ permalink raw reply related
* Re: [PATCH net 2/2] RDS: TCP: Synchrnozize accept() and connect() paths on t_conn_lock.
From: Sowmini Varadhan @ 2016-05-02 16:43 UTC (permalink / raw)
To: Santosh Shilimkar; +Cc: netdev, rds-devel, davem
In-Reply-To: <b5d93d46-5465-ce03-74b1-07136eac0cc9@oracle.com>
On (05/02/16 09:33), Santosh Shilimkar wrote:
> >+ mutex_unlock(&tc->t_conn_lock);
> Just wondering whether the spin_lock() would better here considering
> entry into rds_tcp_conn_connect() & rds_tcp_accept_one() might be
> from softirq context. Ignore it if its not applicable.
It's not from softirq context (both are workqs), but I used a mutex
to follow c_cm_lock (which I considered reusing, given that it
is only IB specific?) But spin_lock vs mutex may not be a big
differentiator here- this is really a one-time start up (corner-case)
issue in the control path.
> > rds_conn_transition(conn, RDS_CONN_DOWN, RDS_CONN_CONNECTING);
> Like patch 1/2, probably we can leverage return value of above.
:
> You probably don't need the local 'conn_state' and below should work.
> if (!rds_conn_connecting(conn) && !rds_conn_up(conn))
see explanation for comment to 1/2.
--Sowmini
^ permalink raw reply
* [PATCH net-next 00/12] Mellanox 100G ethernet SRIOV Upgrades
From: Saeed Mahameed @ 2016-05-02 16:43 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Saeed Mahameed
Hi Dave,
This series introduces new features and upgrades for mlx5 etherenet SRIOV,
while the first patch provides a bug fixes for a compilation issue introduced
buy the previous aRFS series for when CONFIG_RFS_ACCEL=y and CONFIG_MLX5_CORE_EN=n.
SRIOV upgrades:
- Use synchronize_irq instead of the vport events spin_lock
- Fix memory leak in error flow
- Added full VST support
- Spoofcheck support
- Trusted VF promiscuous and allmulti support
VST and Spoofcheck in details:
- Adding Low level firmware commands support for creating ACLs
(Access Control Lists) Flow tables. ACLs are regular flow tables with
the only exception that they are bound to a specific e-Switch vport (VF)
and they can be one of two types
> egress ACL: filters traffic going from e-Switch to VF.
> ingress ACL: filters traffic going from VF to e-Switch.
- Ingress/Egress ACLs (per vport) for VF VST mode filtering.
- Ingress/Egress ACLs (per vport) for VF spoofcheck filtering.
- Ingress/Egress ACLs (per vport) configuration:
> Created only when at least one of (VST, spoofcheck) is configured.
> if (!spoofchk && !vst) allow all traffic. i.e. no ACLs.
> if (spoofchk && vst) allow only untagged traffic with smac=original mac sent from the VF.
> if (spoofchk && !vst) allow only traffic with smac=original mac sent from the VF.
> if (!spoofchk && vst) allow only untagged traffic.
Trusted VF promiscuous and allmulti support in details:
- Added two flow groups for allmulti and promisc VFs to the e-Switch FDB table
> Allmulti group: One rule that forwards any mcast traffic coming from
either uplink or VFs/PF vports.
> Promisc group: One rule that forwards all unmatched traffic coming from uplink.
- Add vport context change event handling for promisc and allmulti
If VF is trusted respect the request and:
> if allmulti request: add the vport to the allmulti group.
and to all other L2 mcast address in the FDB table.
> if promisc request: add the vport to the promisc group.
> Note: A promisc VF can only see traffic that was not explicitly matched to
or requested by any other VF.
Applied on top: 582a1db98892 ("Merge branch 'qed-selftests'")
Thanks,
Saeed.
Maor Gottlieb (1):
net/mlx5e: Fix aRFS compilation dependency
Mohamad Haj Yahia (11):
net/mlx5: Flow steering, Add vport ACL support
net/mlx5: E-Switch, Replace vport spin lock with synchronize_irq()
net/mlx5: E-Switch, Fix error flow memory leak
net/mlx5: E-Switch, Introduce VST vport ingress/egress ACLs
net/mlx5: E-Switch, Vport ingress/egress ACLs rules for VST mode
net/mlx5: E-Switch, Vport ingress/egress ACLs rules for spoofchk
net/mlx5: E-Switch, Enable/disable ACL tables on demand
net/mlx5: E-Switch, Use vport event handler for vport cleanup
net/mlx5: E-Switch, Add promiscuous and allmulti FDB flowtable groups
net/mlx5: E-Switch, Implement promiscuous rx modes vf request handling
net/mlx5: E-Switch, Implement trust vf ndo
drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 8 +
drivers/net/ethernet/mellanox/mlx5/core/Makefile | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 17 +
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 976 +++++++++++++++++++--
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 43 +-
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.c | 33 +
drivers/net/ethernet/mellanox/mlx5/core/fs_cmd.h | 1 +
drivers/net/ethernet/mellanox/mlx5/core/fs_core.c | 85 +-
drivers/net/ethernet/mellanox/mlx5/core/fs_core.h | 7 +-
.../net/ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +
include/linux/mlx5/device.h | 12 +
include/linux/mlx5/driver.h | 2 +
include/linux/mlx5/fs.h | 7 +
13 files changed, 1123 insertions(+), 72 deletions(-)
--
2.8.0
^ permalink raw reply
* [PATCH net-next 03/12] net/mlx5: E-Switch, Replace vport spin lock with synchronize_irq()
From: Saeed Mahameed @ 2016-05-02 16:43 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Or Gerlitz, Tal Alon, Eran Ben Elisha, Mohamad Haj Yahia,
Saeed Mahameed
In-Reply-To: <1462207424-4463-1-git-send-email-saeedm@mellanox.com>
From: Mohamad Haj Yahia <mohamad@mellanox.com>
Vport spin lock can be replaced with synchronize_irq() in the right
place, this will remove the need of locking inside irq context.
Locking in esw_enable_vport is not required since vport events are yet
to be enabled, and at esw_disable_vport it is sufficient to
synchronize_irq() to guarantee no further vport events handlers will be
scheduled.
Signed-off-by: Mohamad Haj Yahia <mohamad@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 11 ++---------
drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 5 -----
2 files changed, 2 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index dd06619..f01903a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -713,7 +713,6 @@ static void esw_enable_vport(struct mlx5_eswitch *esw, int vport_num,
int enable_events)
{
struct mlx5_vport *vport = &esw->vports[vport_num];
- unsigned long flags;
WARN_ON(vport->enabled);
@@ -727,9 +726,7 @@ static void esw_enable_vport(struct mlx5_eswitch *esw, int vport_num,
vport->enabled_events = enable_events;
esw_vport_change_handler(&vport->vport_change_handler);
- spin_lock_irqsave(&vport->lock, flags);
vport->enabled = true;
- spin_unlock_irqrestore(&vport->lock, flags);
arm_vport_context_events_cmd(esw->dev, vport_num, enable_events);
@@ -761,17 +758,16 @@ static void esw_cleanup_vport(struct mlx5_eswitch *esw, u16 vport_num)
static void esw_disable_vport(struct mlx5_eswitch *esw, int vport_num)
{
struct mlx5_vport *vport = &esw->vports[vport_num];
- unsigned long flags;
if (!vport->enabled)
return;
esw_debug(esw->dev, "Disabling vport(%d)\n", vport_num);
/* Mark this vport as disabled to discard new events */
- spin_lock_irqsave(&vport->lock, flags);
vport->enabled = false;
vport->enabled_events = 0;
- spin_unlock_irqrestore(&vport->lock, flags);
+
+ synchronize_irq(mlx5_get_msix_vec(esw->dev, MLX5_EQ_VEC_ASYNC));
mlx5_modify_vport_admin_state(esw->dev,
MLX5_QUERY_VPORT_STATE_IN_OP_MOD_ESW_VPORT,
@@ -894,7 +890,6 @@ int mlx5_eswitch_init(struct mlx5_core_dev *dev)
vport->dev = dev;
INIT_WORK(&vport->vport_change_handler,
esw_vport_change_handler);
- spin_lock_init(&vport->lock);
}
esw->total_vports = total_vports;
@@ -942,10 +937,8 @@ void mlx5_eswitch_vport_event(struct mlx5_eswitch *esw, struct mlx5_eqe *eqe)
}
vport = &esw->vports[vport_num];
- spin_lock(&vport->lock);
if (vport->enabled)
queue_work(esw->work_queue, &vport->vport_change_handler);
- spin_unlock(&vport->lock);
}
/* Vport Administration */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
index 3416a42..ba43451 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
@@ -95,11 +95,6 @@ struct mlx5_vport {
struct hlist_head mc_list[MLX5_L2_ADDR_HASH_SIZE];
struct work_struct vport_change_handler;
- /* This spinlock protects access to vport data, between
- * "esw_vport_disable" and ongoing interrupt "mlx5_eswitch_vport_event"
- * once vport marked as disabled new interrupts are discarded.
- */
- spinlock_t lock; /* vport events sync */
bool enabled;
u16 enabled_events;
};
--
2.8.0
^ permalink raw reply related
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