* Re: [PATCH] net: optimise csum_replace4()
From: Eric Dumazet @ 2014-09-23 11:08 UTC (permalink / raw)
To: Christophe Leroy; +Cc: David S. Miller, linux-kernel, Eric Dumazet, netdev
In-Reply-To: <20140923085433.09C5A1AB26A@localhost.localdomain>
On Tue, 2014-09-23 at 10:54 +0200, Christophe Leroy wrote:
> csum_partial() is a generic function which is not optimised for small fixed
> length calculations, and its use requires to store "from" and "to" values in
> memory while we already have them available in registers. This also has impact,
> especially on RISC processors. In the same spirit as the change done by
> Eric Dumazet on csum_replace2(), this patch rewrites inet_proto_csum_replace4()
> taking into account RFC1624.
>
> I spotted during a NATted tcp transfert that csum_partial() is one of top 5
> consuming functions (around 8%), and the second user of csum_partial() is
> inet_proto_csum_replace4().
>
> I have proposed the same modification to inet_proto_csum_replace4() in another
> patch.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Thanks Christophe, this looks good to me.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Mark Einon @ 2014-09-23 11:06 UTC (permalink / raw)
To: Tobias Klauser; +Cc: devel, gregkh, davem, linux-kernel, netdev
In-Reply-To: <20140923100106.GE4657@distanz.ch>
On Tue, Sep 23, 2014 at 12:01:07PM +0200, Tobias Klauser wrote:
> On 2014-09-23 at 11:46:15 +0200, Mark Einon <mark.einon@gmail.com> wrote:
> > On Tue, Sep 23, 2014 at 09:22:53AM +0200, Tobias Klauser wrote:
> >
> > Hi Tobias,
> >
> > Thanks for the details review. I've replied below -
> >
> > [...]
> > > > +/* et131x_rx_dma_memory_alloc
> > > > + *
> > > > + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
> > > > + * and the Packet Status Ring.
> > > > + */
> > > > +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
> > > > +{
> > > > + u8 id;
> > > > + u32 i, j;
> > > > + u32 bufsize;
> > > > + u32 psr_size;
> > > > + u32 fbr_chunksize;
> > > > + struct rx_ring *rx_ring = &adapter->rx_ring;
> > > > + struct fbr_lookup *fbr;
> > > > +
> > > > + /* Alloc memory for the lookup table */
> > > > + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > > > + if (rx_ring->fbr[0] == NULL)
> > > > + return -ENOMEM;
> > > > + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > > > + if (rx_ring->fbr[1] == NULL)
> > > > + return -ENOMEM;
> > >
> > > If you fail here, et131x_rx_dma_memory_free() will be called by
> > > et131x_adapter_memory_free() in the error handling path which in turn
> > > will access the members of the already allocated rx_ring->fbr[0] (e.g.
> > > ->buffsize). Since it is allocated with kmalloc() these members contain
> > > arbitrary values and cause problems in et131x_rx_dma_memory_free(). I'
> > > suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and
> > > not call et131x_rx_dma_memory_free() in the error handling path. You
> > > might want to check that memory allocation failures are properly handled
> > > in the rest of the driver as well. There are multiple other cases where
> > > et131x_adapter_memory_free() is called on partially
> > > allocated/initialized memory.
> > >
> >
> > I don't think this is the case - the members of rx_ring->fbr[x] are not
> > accessed unless this pointer is non-NULL in et131x_rx_dma_memory_free()
> > (see code snippet below), which is exactly why the kmalloc code above
> > would fail, so buffsize never gets accessed and the code is cleaned up
> > correctly. Actually, for further explanation, this thread discusses
> > these changes:
> >
> > http://www.spinics.net/lists/linux-driver-devel/msg42128.html
>
> Hm, won't rx_ring->fbr[0] be accessed, since it was successfully
> allocated? However, it isn't properly initialized, so at least the
> ->ring_virtaddr will get accessed and if this is non-NULL by accident
> also further members?
I think you have a point there - does a kzalloc(fbr) sort out that problem?
As we would want the !fbr->ring_virtaddr check in the cleanup code below
to be true if allocation fails, and it would be unallocated if the
second fbr alloc fails. The ->ring_virtaddr is NULLed on free, so this
check will always fail subsequently.
I'll take a closer look and provide a patch, thanks.
Mark
> > > > +/* et131x_rx_dma_memory_free - Free all memory allocated within this module */
> > > > +static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
> > > > +{
> >
> > [...]
> >
> > > > + /* Free Free Buffer Rings */
> > > > + for (id = 0; id < NUM_FBRS; id++) {
> > > > + fbr = rx_ring->fbr[id];
> > > > +
> > > > + if (!fbr || !fbr->ring_virtaddr)
> > > > + continue;
> > > > +
> > > > + /* First the packet memory */
> > > > + for (ii = 0; ii < fbr->num_entries / FBR_CHUNKS; ii++) {
> > > > + if (fbr->mem_virtaddrs[ii]) {
> > > > + bufsize = fbr->buffsize * FBR_CHUNKS;
> >
> > [...]
^ permalink raw reply
* Re: [PATCH 2/5] netfilter: nft_rbtree: no need for spinlock from set destroy path
From: Pablo Neira Ayuso @ 2014-09-23 11:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <1411465957.26859.179.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Sep 23, 2014 at 02:52:37AM -0700, Eric Dumazet wrote:
> On Tue, 2014-09-23 at 11:24 +0200, Pablo Neira Ayuso wrote:
> > The sets are released from the rcu callback, after the rule is removed
> > from the chain list, which implies that nfnetlink cannot update the
> > rbtree and no packets are walking on the set anymore. Thus, we can get
> > rid of the spinlock in the set destroy path there.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Reviewied-by: Thomas Graf <tgraf@suug.ch>
> > ---
> > net/netfilter/nft_rbtree.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
> > index e1836ff..46214f2 100644
> > --- a/net/netfilter/nft_rbtree.c
> > +++ b/net/netfilter/nft_rbtree.c
> > @@ -234,13 +234,11 @@ static void nft_rbtree_destroy(const struct nft_set *set)
> > struct nft_rbtree_elem *rbe;
> > struct rb_node *node;
> >
> > - spin_lock_bh(&nft_rbtree_lock);
> > while ((node = priv->root.rb_node) != NULL) {
> > rb_erase(node, &priv->root);
> > rbe = rb_entry(node, struct nft_rbtree_elem, node);
> > nft_rbtree_elem_destroy(set, rbe);
> > }
> > - spin_unlock_bh(&nft_rbtree_lock);
> > }
> >
> > static bool nft_rbtree_estimate(const struct nft_set_desc *desc, u32 features,
>
> BTW, do you know if destroying an rbtree is faster this way, or using
> rb_first() ?
>
> Most cases I see in the kernel use a rb_first instead of taking the
> root.
>
> Examples : (its not an exhaustive list)
>
> net/netfilter/xt_connlimit.c:402
> net/sched/sch_netem.c:380
> net/sched/sch_fq.c:519
> drivers/infiniband/hw/mlx4/cm.c:439
> drivers/iommu/iova.c:324
> drivers/md/dm-thin.c:1491
> drivers/mtd/mtdswap.c:625
> drivers/mtd/ubi/attach.c:636
>
> This might be better for large trees, to get better cache locality,
> but I have no experimental data.
I'll send a follow up patch for nf-next to use rb_first() in that
patch. Thanks Eric.
^ permalink raw reply
* [PATCH v2 net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Eric Dumazet @ 2014-09-23 10:34 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
Eric Dumazet
In-Reply-To: <1411468452-3729-1-git-send-email-edumazet@google.com>
TCP maintains lists of skb in write queue, and in receive queues
(in order and out of order queues)
Scanning these lists both in input and output path usually requires
access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq
These fields are currently in two different cache lines, meaning we
waste lot of memory bandwidth when these queues are big and flows
have either packet drops or packet reorders.
We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
this header is not used in fast path. This allows TCP to search much faster
in the skb lists.
Even with regular flows, we save one cache line miss in fast path.
Thanks to Christoph Paasch for noticing we need to cleanup
skb->cb[] (IPCB/IP6CB) before entering IP stack in tx path.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/tcp.h | 12 ++++++------
net/ipv4/tcp_ipv4.c | 7 +++++++
net/ipv4/tcp_output.c | 5 +++++
net/ipv6/tcp_ipv6.c | 7 +++++++
4 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index a4201ef216e8..4dc6641ee990 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -696,12 +696,6 @@ static inline u32 tcp_skb_timestamp(const struct sk_buff *skb)
* If this grows please adjust skbuff.h:skbuff->cb[xxx] size appropriately.
*/
struct tcp_skb_cb {
- union {
- struct inet_skb_parm h4;
-#if IS_ENABLED(CONFIG_IPV6)
- struct inet6_skb_parm h6;
-#endif
- } header; /* For incoming frames */
__u32 seq; /* Starting sequence number */
__u32 end_seq; /* SEQ + FIN + SYN + datalen */
__u32 tcp_tw_isn; /* isn chosen by tcp_timewait_state_process() */
@@ -720,6 +714,12 @@ struct tcp_skb_cb {
__u8 ip_dsfield; /* IPv4 tos or IPv6 dsfield */
/* 1 byte hole */
__u32 ack_seq; /* Sequence number ACK'd */
+ union {
+ struct inet_skb_parm h4;
+#if IS_ENABLED(CONFIG_IPV6)
+ struct inet6_skb_parm h6;
+#endif
+ } header; /* For incoming frames */
};
#define TCP_SKB_CB(__skb) ((struct tcp_skb_cb *)&((__skb)->cb[0]))
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 28ab90382c01..70c4a21f6f45 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1636,6 +1636,13 @@ int tcp_v4_rcv(struct sk_buff *skb)
th = tcp_hdr(skb);
iph = ip_hdr(skb);
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+ sizeof(struct inet_skb_parm));
+ barrier();
+
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
skb->len - th->doff * 4);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 8c61a7c0c889..6c7949cafd99 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -974,6 +974,11 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
/* Our usage of tstamp should remain private */
skb->tstamp.tv64 = 0;
+
+ /* Cleanup our debris for IP stacks */
+ memset(skb->cb, 0, max(sizeof(struct inet_skb_parm),
+ sizeof(struct inet6_skb_parm)));
+
err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl);
if (likely(err <= 0))
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9400b4326f22..132bac137aed 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1412,6 +1412,13 @@ static int tcp_v6_rcv(struct sk_buff *skb)
th = tcp_hdr(skb);
hdr = ipv6_hdr(skb);
+ /* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+ * barrier() makes sure compiler wont play fool^Waliasing games.
+ */
+ memmove(&TCP_SKB_CB(skb)->header.h6, IP6CB(skb),
+ sizeof(struct inet6_skb_parm));
+ barrier();
+
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
skb->len - th->doff*4);
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH v2 net-next 2/3] ipv6: add a struct inet6_skb_parm param to ipv6_opt_accepted()
From: Eric Dumazet @ 2014-09-23 10:34 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
Eric Dumazet
In-Reply-To: <1411468452-3729-1-git-send-email-edumazet@google.com>
ipv6_opt_accepted() assumes IP6CB(skb) holds the struct inet6_skb_parm
that it needs. Lets not assume this, as TCP stack might use a different
place.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ipv6.h | 3 ++-
net/dccp/ipv6.c | 2 +-
net/ipv6/af_inet6.c | 4 ++--
net/ipv6/syncookies.c | 2 +-
net/ipv6/tcp_ipv6.c | 5 +++--
5 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 7e247e9b8765..97f472012438 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -288,7 +288,8 @@ struct ipv6_txoptions *ipv6_renew_options(struct sock *sk,
struct ipv6_txoptions *ipv6_fixup_options(struct ipv6_txoptions *opt_space,
struct ipv6_txoptions *opt);
-bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb);
+bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb,
+ const struct inet6_skb_parm *opt);
static inline bool ipv6_accept_ra(struct inet6_dev *idev)
{
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 04cb17d4b0ce..ad2acfe1ca61 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -404,7 +404,7 @@ static int dccp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
- if (ipv6_opt_accepted(sk, skb) ||
+ if (ipv6_opt_accepted(sk, skb, IP6CB(skb)) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index e4865a3ebe1d..34f726f59814 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -672,10 +672,10 @@ int inet6_sk_rebuild_header(struct sock *sk)
}
EXPORT_SYMBOL_GPL(inet6_sk_rebuild_header);
-bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb)
+bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb,
+ const struct inet6_skb_parm *opt)
{
const struct ipv6_pinfo *np = inet6_sk(sk);
- const struct inet6_skb_parm *opt = IP6CB(skb);
if (np->rxopt.all) {
if ((opt->hop && (np->rxopt.bits.hopopts ||
diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c
index c643dc907ce7..9a2838e93cc5 100644
--- a/net/ipv6/syncookies.c
+++ b/net/ipv6/syncookies.c
@@ -203,7 +203,7 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb)
ireq->ir_num = ntohs(th->dest);
ireq->ir_v6_rmt_addr = ipv6_hdr(skb)->saddr;
ireq->ir_v6_loc_addr = ipv6_hdr(skb)->daddr;
- if (ipv6_opt_accepted(sk, skb) ||
+ if (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||
np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim) {
atomic_inc(&skb->users);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index de51a88bec6f..9400b4326f22 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -742,7 +742,8 @@ static void tcp_v6_init_req(struct request_sock *req, struct sock *sk,
ireq->ir_iif = inet6_iif(skb);
if (!TCP_SKB_CB(skb)->tcp_tw_isn &&
- (ipv6_opt_accepted(sk, skb) || np->rxopt.bits.rxinfo ||
+ (ipv6_opt_accepted(sk, skb, &TCP_SKB_CB(skb)->header.h6) ||
+ np->rxopt.bits.rxinfo ||
np->rxopt.bits.rxoinfo || np->rxopt.bits.rxhlim ||
np->rxopt.bits.rxohlim || np->repflow)) {
atomic_inc(&skb->users);
@@ -1367,7 +1368,7 @@ ipv6_pktoptions:
np->rcv_flowinfo = ip6_flowinfo(ipv6_hdr(opt_skb));
if (np->repflow)
np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
- if (ipv6_opt_accepted(sk, opt_skb)) {
+ if (ipv6_opt_accepted(sk, opt_skb, &TCP_SKB_CB(opt_skb)->header.h6)) {
skb_set_owner_r(opt_skb, sk);
opt_skb = xchg(&np->pktoptions, opt_skb);
} else {
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH v2 net-next 1/3] ipv4: rename ip_options_echo to __ip_options_echo()
From: Eric Dumazet @ 2014-09-23 10:34 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
Eric Dumazet
In-Reply-To: <1411468452-3729-1-git-send-email-edumazet@google.com>
ip_options_echo() assumes struct ip_options is provided in &IPCB(skb)->opt
Lets break this assumption, but provide a helper to not change all call points.
ip_send_unicast_reply() gets a new struct ip_options pointer.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/net/ip.h | 15 ++++++++++++---
net/ipv4/ip_options.c | 6 ++----
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/tcp_ipv4.c | 10 ++++++----
4 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 14bfc8e1bcf9..c90e3357d589 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -180,8 +180,10 @@ static inline __u8 ip_reply_arg_flowi_flags(const struct ip_reply_arg *arg)
return (arg->flags & IP_REPLY_ARG_NOSRCCHECK) ? FLOWI_FLAG_ANYSRC : 0;
}
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
- __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+ const struct ip_options *sopt,
+ __be32 daddr, __be32 saddr,
+ const struct ip_reply_arg *arg,
unsigned int len);
#define IP_INC_STATS(net, field) SNMP_INC_STATS64((net)->mib.ip_statistics, field)
@@ -511,7 +513,14 @@ int ip_forward(struct sk_buff *skb);
void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
__be32 daddr, struct rtable *rt, int is_frag);
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb);
+
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+ const struct ip_options *sopt);
+static inline int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+{
+ return __ip_options_echo(dopt, skb, &IPCB(skb)->opt);
+}
+
void ip_options_fragment(struct sk_buff *skb);
int ip_options_compile(struct net *net, struct ip_options *opt,
struct sk_buff *skb);
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ad382499bace..5b3d91be2db0 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -87,17 +87,15 @@ void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
* NOTE: dopt cannot point to skb.
*/
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+ const struct ip_options *sopt)
{
- const struct ip_options *sopt;
unsigned char *sptr, *dptr;
int soffset, doffset;
int optlen;
memset(dopt, 0, sizeof(struct ip_options));
- sopt = &(IPCB(skb)->opt);
-
if (sopt->optlen == 0)
return 0;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 215af2b155cb..c8fa62476461 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1522,8 +1522,10 @@ static DEFINE_PER_CPU(struct inet_sock, unicast_sock) = {
.uc_ttl = -1,
};
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
- __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+ const struct ip_options *sopt,
+ __be32 daddr, __be32 saddr,
+ const struct ip_reply_arg *arg,
unsigned int len)
{
struct ip_options_data replyopts;
@@ -1534,7 +1536,7 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
struct sock *sk;
struct inet_sock *inet;
- if (ip_options_echo(&replyopts.opt.opt, skb))
+ if (__ip_options_echo(&replyopts.opt.opt, skb, sopt))
return;
ipc.addr = daddr;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3b2e49cb2b61..28ab90382c01 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -681,8 +681,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
net = dev_net(skb_dst(skb)->dev);
arg.tos = ip_hdr(skb)->tos;
- ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
- ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+ ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+ ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+ &arg, arg.iov[0].iov_len);
TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
@@ -764,8 +765,9 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
if (oif)
arg.bound_dev_if = oif;
arg.tos = tos;
- ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
- ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+ ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+ ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+ &arg, arg.iov[0].iov_len);
TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
}
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* [PATCH v2 net-next 0/3] tcp: better TCP_SKB_CB layout
From: Eric Dumazet @ 2014-09-23 10:34 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
Eric Dumazet
TCP had the assumption that IPCB and IP6CB are first members of skb->cb[]
This is fine, except that IPCB/IP6CB are used in TCP for a very short time
in input path.
What really matters for TCP stack is to get skb->next,
TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq in the same cache line.
skb that are immediately consumed do not care because whole skb->cb[] is
hot in cpu cache, while skb that sit in wocket write queue or receive queues
do not need TCP_SKB_CB(skb)->header at all.
This patch set implements the prereq for IPv4, IPv6, and TCP to make this
possible. This makes TCP more efficient.
Eric Dumazet (3):
ipv4: rename ip_options_echo to __ip_options_echo()
ipv6: add a struct inet6_skb_parm param to ipv6_opt_accepted()
tcp: better TCP_SKB_CB layout to reduce cache line misses
include/net/ip.h | 15 ++++++++++++---
include/net/ipv6.h | 3 ++-
include/net/tcp.h | 12 ++++++------
net/dccp/ipv6.c | 2 +-
net/ipv4/ip_options.c | 6 ++----
net/ipv4/ip_output.c | 8 +++++---
net/ipv4/tcp_ipv4.c | 17 +++++++++++++----
net/ipv4/tcp_output.c | 5 +++++
net/ipv6/af_inet6.c | 4 ++--
net/ipv6/syncookies.c | 2 +-
net/ipv6/tcp_ipv6.c | 12 ++++++++++--
11 files changed, 59 insertions(+), 27 deletions(-)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Thomas Graf @ 2014-09-23 10:11 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Tom Herbert, Alexei Starovoitov, Jiri Pirko, John Fastabend,
Jamal Hadi Salim, netdev@vger.kernel.org, David S. Miller,
Neil Horman, Andy Gospodarek, Daniel Borkmann, Or Gerlitz,
Jesse Gross, Pravin Shelar, Andy Zhou, Ben Hutchings,
Stephen Hemminger, Jeff Kirsher, Vladislav Yasevich, Cong Wang,
Eric Dumazet, Scott Feldman, Florian Fainelli, Roop
In-Reply-To: <20140923041130.GA10808@gospo.home.greyhouse.net>
On 09/23/14 at 12:11am, Andy Gospodarek wrote:
> There are clearly some that are most interested in how an eSwitch on an
> SR-IOV capable NIC be controlled can provide traditional forwarding help
> as well as offload the various technologies they hope to terminate
> at/inside their endpoint (host/guest/container) -- Thomas's _simple_
> use-case demonstrates this. ;) This is a logical extention/increase in
> functionality that is offered in many eSwitches that was previously
> hidden from the user with the first generation SR-IOV capable network
> devices on hosts/servers.
I think we can define this more broadly and state that providing RX
steering capabilities to identify a guest in the NIC allows to
directly map packets into a memory region shared between host and
guest. Not a new concept at all but the existing dMAC and VLAN rx
filtering is just too limiting. We require a programmable API with
support for encap and encryption. SR-IOV is a hardware assisted form
of that which can expedite the guest to guest path on a host.
^ permalink raw reply
* Re: BCM4313 & brcmsmac & 3.12: only semi-working?
From: Michael Tokarev @ 2014-09-23 10:04 UTC (permalink / raw)
To: Arend van Spriel; +Cc: brcm80211-dev-list, linux-wireless, netdev
In-Reply-To: <541EEF17.7020806@msgid.tls.msk.ru>
21.09.2014 19:30, Michael Tokarev wrote:
> Namely, after resume, the card does not work. ARP works, ping and
> DNS sometimes/somewhat work, inital TCP connection establisment
> works, but eg http download does not work, it stalls almost
> immediately.
It does not work at all anymore again. Not only after resume but also
after could power up, it stalls right at start of wget for example.
> http://www.corpit.ru/mjt/tmp/brcmsmac-4313-trace-20140921.dat.gz -- this
> is a trace collected after resuming from suspend-to-disk
And here -- http://www.corpit.ru/mjt/tmp/brcmsmac-4313-trace-20140923.dat.gz
is another trace, after insmod brcmutils & brcmsmac, starting trace-cmd record
(with network-manager running in the background), and running a wget on
http://ip-add-ress-of-the-access-point/somefile (which immediately stalls,
showing minimal progress once in a while only).
Hopefully this will help to find the problem. And for the record, I'm
_again_ without the wifi network. But unlike of the previous situation,
I can at least buy some other, better supported wifi adapter now, because
this laptop does not have a bios lock (is 6235ANHMW any good?)
Thanks,
/mjt
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Tobias Klauser @ 2014-09-23 10:01 UTC (permalink / raw)
To: Mark Einon; +Cc: devel, gregkh, davem, linux-kernel, netdev
In-Reply-To: <20140923094614.GA5130@leicester.auvation.com>
On 2014-09-23 at 11:46:15 +0200, Mark Einon <mark.einon@gmail.com> wrote:
> On Tue, Sep 23, 2014 at 09:22:53AM +0200, Tobias Klauser wrote:
>
> Hi Tobias,
>
> Thanks for the details review. I've replied below -
>
> [...]
> > > +/* et131x_rx_dma_memory_alloc
> > > + *
> > > + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
> > > + * and the Packet Status Ring.
> > > + */
> > > +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
> > > +{
> > > + u8 id;
> > > + u32 i, j;
> > > + u32 bufsize;
> > > + u32 psr_size;
> > > + u32 fbr_chunksize;
> > > + struct rx_ring *rx_ring = &adapter->rx_ring;
> > > + struct fbr_lookup *fbr;
> > > +
> > > + /* Alloc memory for the lookup table */
> > > + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > > + if (rx_ring->fbr[0] == NULL)
> > > + return -ENOMEM;
> > > + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > > + if (rx_ring->fbr[1] == NULL)
> > > + return -ENOMEM;
> >
> > If you fail here, et131x_rx_dma_memory_free() will be called by
> > et131x_adapter_memory_free() in the error handling path which in turn
> > will access the members of the already allocated rx_ring->fbr[0] (e.g.
> > ->buffsize). Since it is allocated with kmalloc() these members contain
> > arbitrary values and cause problems in et131x_rx_dma_memory_free(). I'
> > suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and
> > not call et131x_rx_dma_memory_free() in the error handling path. You
> > might want to check that memory allocation failures are properly handled
> > in the rest of the driver as well. There are multiple other cases where
> > et131x_adapter_memory_free() is called on partially
> > allocated/initialized memory.
> >
>
> I don't think this is the case - the members of rx_ring->fbr[x] are not
> accessed unless this pointer is non-NULL in et131x_rx_dma_memory_free()
> (see code snippet below), which is exactly why the kmalloc code above
> would fail, so buffsize never gets accessed and the code is cleaned up
> correctly. Actually, for further explanation, this thread discusses
> these changes:
>
> http://www.spinics.net/lists/linux-driver-devel/msg42128.html
Hm, won't rx_ring->fbr[0] be accessed, since it was successfully
allocated? However, it isn't properly initialized, so at least the
->ring_virtaddr will get accessed and if this is non-NULL by accident
also further members?
> > > +/* et131x_rx_dma_memory_free - Free all memory allocated within this module */
> > > +static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
> > > +{
>
> [...]
>
> > > + /* Free Free Buffer Rings */
> > > + for (id = 0; id < NUM_FBRS; id++) {
> > > + fbr = rx_ring->fbr[id];
> > > +
> > > + if (!fbr || !fbr->ring_virtaddr)
> > > + continue;
> > > +
> > > + /* First the packet memory */
> > > + for (ii = 0; ii < fbr->num_entries / FBR_CHUNKS; ii++) {
> > > + if (fbr->mem_virtaddrs[ii]) {
> > > + bufsize = fbr->buffsize * FBR_CHUNKS;
>
> [...]
>
> > > +static SIMPLE_DEV_PM_OPS(et131x_pm_ops, et131x_suspend, et131x_resume);
> > > +#define ET131X_PM_OPS (&et131x_pm_ops)
> > > +#else
> > > +#define ET131X_PM_OPS NULL
> > > +#endif
> >
> > No need for the #define here, just assigne et131x_pm_ops to .driver.pm
> > directly, its members will be NULL and thus never called. Also, you can
> > make et131x_pm_ops const.
>
> Ok, I can change this.
>
> Btw, this appears to be a fairly standard way of using .driver.pm among
> ethernet drivers, e.g. see 3com/3c59x.c, atheros, marvell... - perhaps
> there is a case for changing all instances of this code?
Yes, this would probably make sense. Looks like a good job for a
coccinelle patch.
^ permalink raw reply
* Re: [PATCH 2/5] netfilter: nft_rbtree: no need for spinlock from set destroy path
From: Eric Dumazet @ 2014-09-23 9:52 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel, davem, netdev
In-Reply-To: <1411464288-18069-3-git-send-email-pablo@netfilter.org>
On Tue, 2014-09-23 at 11:24 +0200, Pablo Neira Ayuso wrote:
> The sets are released from the rcu callback, after the rule is removed
> from the chain list, which implies that nfnetlink cannot update the
> rbtree and no packets are walking on the set anymore. Thus, we can get
> rid of the spinlock in the set destroy path there.
>
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Reviewied-by: Thomas Graf <tgraf@suug.ch>
> ---
> net/netfilter/nft_rbtree.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
> index e1836ff..46214f2 100644
> --- a/net/netfilter/nft_rbtree.c
> +++ b/net/netfilter/nft_rbtree.c
> @@ -234,13 +234,11 @@ static void nft_rbtree_destroy(const struct nft_set *set)
> struct nft_rbtree_elem *rbe;
> struct rb_node *node;
>
> - spin_lock_bh(&nft_rbtree_lock);
> while ((node = priv->root.rb_node) != NULL) {
> rb_erase(node, &priv->root);
> rbe = rb_entry(node, struct nft_rbtree_elem, node);
> nft_rbtree_elem_destroy(set, rbe);
> }
> - spin_unlock_bh(&nft_rbtree_lock);
> }
>
> static bool nft_rbtree_estimate(const struct nft_set_desc *desc, u32 features,
BTW, do you know if destroying an rbtree is faster this way, or using
rb_first() ?
Most cases I see in the kernel use a rb_first instead of taking the
root.
Examples : (its not an exhaustive list)
net/netfilter/xt_connlimit.c:402
net/sched/sch_netem.c:380
net/sched/sch_fq.c:519
drivers/infiniband/hw/mlx4/cm.c:439
drivers/iommu/iova.c:324
drivers/md/dm-thin.c:1491
drivers/mtd/mtdswap.c:625
drivers/mtd/ubi/attach.c:636
This might be better for large trees, to get better cache locality,
but I have no experimental data.
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Thomas Graf @ 2014-09-23 9:52 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexei Starovoitov, Jiri Pirko, John Fastabend, Jamal Hadi Salim,
netdev@vger.kernel.org, David S. Miller, Neil Horman,
Andy Gospodarek, Daniel Borkmann, Or Gerlitz, Jesse Gross,
Pravin Shelar, Andy Zhou, Ben Hutchings, Stephen Hemminger,
Jeff Kirsher, Vladislav Yasevich, Cong Wang, Eric Dumazet,
Scott Feldman, Florian Fainelli, Roopa Prabhu
In-Reply-To: <CA+mtBx-mJE6SW2bqf3_t6iu=o5FY1WAF1ByeZpDJnc+6fpK6nA@mail.gmail.com>
On 09/22/14 at 07:16pm, Tom Herbert wrote:
> Turn on UDP RSS on the device and I bet you'll see those differences
> go away! Once we moved to UDP encapsulation, there's really little
> value in looking at inner headers for RSS or ECMP, this should be
> sufficient. Sure someone might want to parse the inner headers for
> some sort of advanced RX steering, but again this implies rx-filtering
> and not switch functionality.
Agreed. The reason we discuss this in the context of this thread is
because the required rx-filtering capabilities seem to be introduced
in the form of (adapted) switch chip integrations onto NICs. In that
sense, OVS is essentially doing advanced RX steering in software.
I agree that switch functionality (whatever that specifically implies)
is not strictly required for the host if you consider queue
redirection as part of RX steering. The exception here would be use
of SR-IOV which could be highly interesting for corner cases if
combined with smart elephant guest detection. A classic example would
be NFV deployed in a virtualized environment, i.e. a virtual firewall
or DPI application serving a bunch of guests.
> If this is something that doesn't require any model change to the
> stack and is just a clever backend for rx-filters or tc, then I'm fine
> with that!
I haven't seen any model change proposed. I'm most certainly not
advocating that. Anyone who can live a model change might as well
just stick to SnabbSwitch or DPDK.
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Mark Einon @ 2014-09-23 9:51 UTC (permalink / raw)
To: Angus Gibson; +Cc: devel, netdev, linux-kernel
In-Reply-To: <20140922235653.GA17359@angus-mbp>
On Tue, Sep 23, 2014 at 09:56:53AM +1000, Angus Gibson wrote:
> > +config NET_VENDOR_AGERE
> > + bool "Agere devices"
> > + default y
> > + depends on PCI
> > + ---help---
> > + If you have a network (Ethernet) card belonging to this class, say =
> > Y
> > + and read the Ethernet-HOWTO, available from
> > + <http://www.tldp.org/docs.html#howto>.
> > +
> > + Note that the answer to this question doesn't directly affect the
> > + kernel: saying N will just cause the configurator to skip all
> > + the questions about Atheros devices. If you say Y, you will be aske=
> should this be "Agere devices" instead?
Yes it should - well spotted, thanks - I'll change this in v2.
Mark
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Mark Einon @ 2014-09-23 9:50 UTC (permalink / raw)
To: Joe Perches; +Cc: devel, gregkh, netdev, davem, linux-kernel
In-Reply-To: <1411437445.2952.61.camel@joe-AO725>
On Mon, Sep 22, 2014 at 06:57:25PM -0700, Joe Perches wrote:
> On Mon, 2014-09-22 at 22:28 +0100, Mark Einon wrote:
> > This patch moves the et131x gigabit ethernet driver from drivers/staging
> > to drivers/net/ethernet/agere.
> >
> > There are no known issues at this time.
>
> This doesn't apply to -next or -linux
Hi Joe,
Thanks for the review.
Yes, there are a further (minor) 8 patches by me on the driver-devel
list that are needed that this patch is based upon, as per my comment
below. As the merge window is close, I've put this out an an RFC to
get feedback ASAP.
>
> []
> > This patch will only apply once the last few pending changes
> > make their way from staging-next to linux-next, but posting
> > now in the hope that feedback can be given and this change can
> > make it in before the next merge window.n yo
>
> When you do post the actual patch, can you please use
> "git format-patch -M" so that the patch is significantly
> smaller and the renames are obvious?
Will do, thanks.
Mark
^ permalink raw reply
* Re: [RFC PATCH linux-next] et131x: Promote staging et131x driver to drivers/net
From: Mark Einon @ 2014-09-23 9:46 UTC (permalink / raw)
To: Tobias Klauser; +Cc: devel, gregkh, davem, linux-kernel, netdev
In-Reply-To: <20140923072251.GD4657@distanz.ch>
On Tue, Sep 23, 2014 at 09:22:53AM +0200, Tobias Klauser wrote:
Hi Tobias,
Thanks for the details review. I've replied below -
[...]
> > +/* et131x_rx_dma_memory_alloc
> > + *
> > + * Allocates Free buffer ring 1 for sure, free buffer ring 0 if required,
> > + * and the Packet Status Ring.
> > + */
> > +static int et131x_rx_dma_memory_alloc(struct et131x_adapter *adapter)
> > +{
> > + u8 id;
> > + u32 i, j;
> > + u32 bufsize;
> > + u32 psr_size;
> > + u32 fbr_chunksize;
> > + struct rx_ring *rx_ring = &adapter->rx_ring;
> > + struct fbr_lookup *fbr;
> > +
> > + /* Alloc memory for the lookup table */
> > + rx_ring->fbr[0] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > + if (rx_ring->fbr[0] == NULL)
> > + return -ENOMEM;
> > + rx_ring->fbr[1] = kmalloc(sizeof(*fbr), GFP_KERNEL);
> > + if (rx_ring->fbr[1] == NULL)
> > + return -ENOMEM;
>
> If you fail here, et131x_rx_dma_memory_free() will be called by
> et131x_adapter_memory_free() in the error handling path which in turn
> will access the members of the already allocated rx_ring->fbr[0] (e.g.
> ->buffsize). Since it is allocated with kmalloc() these members contain
> arbitrary values and cause problems in et131x_rx_dma_memory_free(). I'
> suggest to do the cleanup (i.e. free rx_ring->fbr[0] directly here and
> not call et131x_rx_dma_memory_free() in the error handling path. You
> might want to check that memory allocation failures are properly handled
> in the rest of the driver as well. There are multiple other cases where
> et131x_adapter_memory_free() is called on partially
> allocated/initialized memory.
>
I don't think this is the case - the members of rx_ring->fbr[x] are not
accessed unless this pointer is non-NULL in et131x_rx_dma_memory_free()
(see code snippet below), which is exactly why the kmalloc code above
would fail, so buffsize never gets accessed and the code is cleaned up
correctly. Actually, for further explanation, this thread discusses
these changes:
http://www.spinics.net/lists/linux-driver-devel/msg42128.html
> > +/* et131x_rx_dma_memory_free - Free all memory allocated within this module */
> > +static void et131x_rx_dma_memory_free(struct et131x_adapter *adapter)
> > +{
[...]
> > + /* Free Free Buffer Rings */
> > + for (id = 0; id < NUM_FBRS; id++) {
> > + fbr = rx_ring->fbr[id];
> > +
> > + if (!fbr || !fbr->ring_virtaddr)
> > + continue;
> > +
> > + /* First the packet memory */
> > + for (ii = 0; ii < fbr->num_entries / FBR_CHUNKS; ii++) {
> > + if (fbr->mem_virtaddrs[ii]) {
> > + bufsize = fbr->buffsize * FBR_CHUNKS;
[...]
> > +static SIMPLE_DEV_PM_OPS(et131x_pm_ops, et131x_suspend, et131x_resume);
> > +#define ET131X_PM_OPS (&et131x_pm_ops)
> > +#else
> > +#define ET131X_PM_OPS NULL
> > +#endif
>
> No need for the #define here, just assigne et131x_pm_ops to .driver.pm
> directly, its members will be NULL and thus never called. Also, you can
> make et131x_pm_ops const.
Ok, I can change this.
Btw, this appears to be a fairly standard way of using .driver.pm among
ethernet drivers, e.g. see 3com/3c59x.c, atheros, marvell... - perhaps
there is a case for changing all instances of this code?
> > +
> > +/* et131x_isr - The Interrupt Service Routine for the driver.
> > + * @irq: the IRQ on which the interrupt was received.
> > + * @dev_id: device-specific info (here a pointer to a net_device struct)
> > + *
> > + * Returns a value indicating if the interrupt was handled.
> > + */
> > +static irqreturn_t et131x_isr(int irq, void *dev_id)
> > +{
> > + bool handled = true;
> > + bool enable_interrupts = true;
> > + struct net_device *netdev = (struct net_device *)dev_id;
>
> No need to cast a void *.
>
> [...]
>
> > +static const struct pci_device_id et131x_pci_table[] = {
> > + { PCI_VDEVICE(ATT, ET131X_PCI_DEVICE_ID_GIG), 0UL},
> > + { PCI_VDEVICE(ATT, ET131X_PCI_DEVICE_ID_FAST), 0UL},
> > + {0,}
>
> Nit: Space after opening curly brace.
Ok, I'll change both of these, thanks.
Mark
^ permalink raw reply
* Re: [PATCH] brcmfmac: Fix off by one bug in brcmf_count_20mhz_channels()
From: Arend van Spriel @ 2014-09-23 9:33 UTC (permalink / raw)
To: Emil Goode
Cc: Brett Rudley, Franky (Zhenhui) Lin, Hante Meuleman,
John W. Linville, Pieter-Paul Giesberts, Daniel Kim,
linux-wireless, brcm80211-dev-list, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <20140922230833.GB10356@lianli>
On 09/23/14 01:08, Emil Goode wrote:
> Hello Arend,
>
> Sorry for the late reply. I have attached a kernel log with brcmfmac
> debugging enabled (without my patch applied).
>
> Let me know if I can provide any other useful information.
No problem, Emil
I was wondering what was returned on "chanspecs" query. So 17 channel
configs which is expected.
Regards,
Arend
> Best regards,
>
> Emil
>
> On Mon, Sep 22, 2014 at 11:56:43AM +0200, Arend van Spriel wrote:
>> On 09/21/14 00:58, Emil Goode wrote:
>>> In the brcmf_count_20mhz_channels function we are looping through a list
>>> of channels received from firmware. Since the index of the first channel
>>> is 0 the condition leads to an off by one bug. This is causing us to hit
>>> the WARN_ON_ONCE(1) calls in the brcmu_d11n_decchspec function, which is
>>> how I discovered the bug.
>>
>> The fix is fine. Would like to know what exactly is going wrong. Can you
>> provide a kernel log with brcmfmac debugging enabled, ie. insmod brcmfmac.ko
>> debug=0x1416
>>
>> Regards,
>> Arend
>>
>>> Introduced by:
>>> commit b48d891676f756d48b4d0ee131e4a7a5d43ca417
>>> ("brcmfmac: rework wiphy structure setup")
>>>
>>> Signed-off-by: Emil Goode<emilgoode@gmail.com>
>>> ---
>>> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>>> index 02fe706..93b5dd9 100644
>>> --- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
>>> @@ -4918,7 +4918,7 @@ static void brcmf_count_20mhz_channels(struct brcmf_cfg80211_info *cfg,
>>> struct brcmu_chan ch;
>>> int i;
>>>
>>> - for (i = 0; i<= total; i++) {
>>> + for (i = 0; i< total; i++) {
>>> ch.chspec = (u16)le32_to_cpu(chlist->element[i]);
>>> cfg->d11inf.decchspec(&ch);
>>>
>>
^ permalink raw reply
* Re: [PATCHv4 1/1] bluetooth: Check for SCO type before setting retransmission effort
From: Marcel Holtmann @ 2014-09-23 9:32 UTC (permalink / raw)
To: Bernhard Thaler
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
BlueZ development, Network Development, linux-kernel
In-Reply-To: <1411462867-4010-1-git-send-email-bernhard.thaler@r-it.at>
Hi Bernhard,
> SCO connection cannot be setup to devices that do not support retransmission.
> Patch based on http://permalink.gmane.org/gmane.linux.bluez.kernel/7779 and
> adapted for this kernel version.
> Code changed to check SCO/eSCO type before setting retransmission effort
> and max. latency. The purpose of the patch is to support older devices not
> capable of eSCO.
>
> Tested on Blackberry 655+ headset which does not support retransmission.
> Credits go to Alexander Sommerhuber.
>
> Signed-off-by: Bernhard Thaler <bernhard.thaler@r-it.at>
> ---
> net/bluetooth/hci_conn.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* [PATCH 2/5] netfilter: nft_rbtree: no need for spinlock from set destroy path
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>
The sets are released from the rcu callback, after the rule is removed
from the chain list, which implies that nfnetlink cannot update the
rbtree and no packets are walking on the set anymore. Thus, we can get
rid of the spinlock in the set destroy path there.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Reviewied-by: Thomas Graf <tgraf@suug.ch>
---
net/netfilter/nft_rbtree.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/netfilter/nft_rbtree.c b/net/netfilter/nft_rbtree.c
index e1836ff..46214f2 100644
--- a/net/netfilter/nft_rbtree.c
+++ b/net/netfilter/nft_rbtree.c
@@ -234,13 +234,11 @@ static void nft_rbtree_destroy(const struct nft_set *set)
struct nft_rbtree_elem *rbe;
struct rb_node *node;
- spin_lock_bh(&nft_rbtree_lock);
while ((node = priv->root.rb_node) != NULL) {
rb_erase(node, &priv->root);
rbe = rb_entry(node, struct nft_rbtree_elem, node);
nft_rbtree_elem_destroy(set, rbe);
}
- spin_unlock_bh(&nft_rbtree_lock);
}
static bool nft_rbtree_estimate(const struct nft_set_desc *desc, u32 features,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/5] nf pull request for net
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi David,
This series contains netfilter fixes for net, they are:
1) Fix lockdep splat in nft_hash when releasing sets from the
rcu_callback context. We don't the mutex there anymore.
2) Remove unnecessary spinlock_bh in the destroy path of the nf_tables
rbtree set type from rcu_callback context.
3) Fix another lockdep splat in rhashtable. None of the callers hold
a mutex when calling rhashtable_destroy.
4) Fix duplicated error reporting from nfnetlink when aborting and
replaying a batch.
5) Fix a Kconfig issue reported by kbuild robot.
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks!
----------------------------------------------------------------
The following changes since commit bec6bfb2437f4676dbaaacba6019e9dafef18962:
amd-xgbe: Fix initialization of the wrong spin lock (2014-09-02 14:03:37 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
for you to fetch changes up to 679ab4ddbdfab8af39104e63819db71f428aefd9:
netfilter: xt_TPROXY: undefined reference to `udp6_lib_lookup' (2014-09-07 17:25:16 +0200)
----------------------------------------------------------------
Pablo Neira Ayuso (5):
netfilter: nft_hash: no need for rcu in the hash set destroy path
netfilter: nft_rbtree: no need for spinlock from set destroy path
rhashtable: fix lockdep splat in rhashtable_destroy()
netfilter: nfnetlink: deliver netlink errors on batch completion
netfilter: xt_TPROXY: undefined reference to `udp6_lib_lookup'
lib/rhashtable.c | 8 +++---
net/netfilter/Kconfig | 1 +
net/netfilter/nfnetlink.c | 64 +++++++++++++++++++++++++++++++++++++++++++-
net/netfilter/nft_hash.c | 12 +++++----
net/netfilter/nft_rbtree.c | 2 --
5 files changed, 75 insertions(+), 12 deletions(-)
^ permalink raw reply
* [PATCH 5/5] netfilter: xt_TPROXY: undefined reference to `udp6_lib_lookup'
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>
CONFIG_IPV6=m
CONFIG_NETFILTER_XT_TARGET_TPROXY=y
net/built-in.o: In function `nf_tproxy_get_sock_v6.constprop.11':
>> xt_TPROXY.c:(.text+0x583a1): undefined reference to `udp6_lib_lookup'
net/built-in.o: In function `tproxy_tg_init':
>> xt_TPROXY.c:(.init.text+0x1dc3): undefined reference to `nf_defrag_ipv6_enable'
This fix is similar to 1a5bbfc ("netfilter: Fix build errors with
xt_socket.c").
Reported-by: kbuild test robot <fengguang.wu@intel.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/Kconfig b/net/netfilter/Kconfig
index 4bef6eb..831f960 100644
--- a/net/netfilter/Kconfig
+++ b/net/netfilter/Kconfig
@@ -839,6 +839,7 @@ config NETFILTER_XT_TARGET_TPROXY
tristate '"TPROXY" target transparent proxying support'
depends on NETFILTER_XTABLES
depends on NETFILTER_ADVANCED
+ depends on (IPV6 || IPV6=n)
depends on IP_NF_MANGLE
select NF_DEFRAG_IPV4
select NF_DEFRAG_IPV6 if IP6_NF_IPTABLES
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/5] netfilter: nfnetlink: deliver netlink errors on batch completion
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>
We have to wait until the full batch has been processed to deliver the
netlink error messages to userspace. Otherwise, we may deliver
duplicated errors to userspace in case that we need to abort and replay
the transaction if any of the required modules needs to be autoloaded.
A simple way to reproduce this (assumming nft_meta is not loaded) with
the following test file:
add table filter
add chain filter test
add chain bad test # intentional wrong unexistent table
add rule filter test meta mark 0
Then, when trying to load the batch:
# nft -f test
test:4:1-19: Error: Could not process rule: No such file or directory
add chain bad test
^^^^^^^^^^^^^^^^^^^
test:4:1-19: Error: Could not process rule: No such file or directory
add chain bad test
^^^^^^^^^^^^^^^^^^^
The error is reported twice, once when the batch is aborted due to
missing nft_meta and another when it is fully processed.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nfnetlink.c | 64 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/net/netfilter/nfnetlink.c b/net/netfilter/nfnetlink.c
index c138b8f..f37f071 100644
--- a/net/netfilter/nfnetlink.c
+++ b/net/netfilter/nfnetlink.c
@@ -222,6 +222,51 @@ replay:
}
}
+struct nfnl_err {
+ struct list_head head;
+ struct nlmsghdr *nlh;
+ int err;
+};
+
+static int nfnl_err_add(struct list_head *list, struct nlmsghdr *nlh, int err)
+{
+ struct nfnl_err *nfnl_err;
+
+ nfnl_err = kmalloc(sizeof(struct nfnl_err), GFP_KERNEL);
+ if (nfnl_err == NULL)
+ return -ENOMEM;
+
+ nfnl_err->nlh = nlh;
+ nfnl_err->err = err;
+ list_add_tail(&nfnl_err->head, list);
+
+ return 0;
+}
+
+static void nfnl_err_del(struct nfnl_err *nfnl_err)
+{
+ list_del(&nfnl_err->head);
+ kfree(nfnl_err);
+}
+
+static void nfnl_err_reset(struct list_head *err_list)
+{
+ struct nfnl_err *nfnl_err, *next;
+
+ list_for_each_entry_safe(nfnl_err, next, err_list, head)
+ nfnl_err_del(nfnl_err);
+}
+
+static void nfnl_err_deliver(struct list_head *err_list, struct sk_buff *skb)
+{
+ struct nfnl_err *nfnl_err, *next;
+
+ list_for_each_entry_safe(nfnl_err, next, err_list, head) {
+ netlink_ack(skb, nfnl_err->nlh, nfnl_err->err);
+ nfnl_err_del(nfnl_err);
+ }
+}
+
static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
u_int16_t subsys_id)
{
@@ -230,6 +275,7 @@ static void nfnetlink_rcv_batch(struct sk_buff *skb, struct nlmsghdr *nlh,
const struct nfnetlink_subsystem *ss;
const struct nfnl_callback *nc;
bool success = true, done = false;
+ static LIST_HEAD(err_list);
int err;
if (subsys_id >= NFNL_SUBSYS_COUNT)
@@ -287,6 +333,7 @@ replay:
type = nlh->nlmsg_type;
if (type == NFNL_MSG_BATCH_BEGIN) {
/* Malformed: Batch begin twice */
+ nfnl_err_reset(&err_list);
success = false;
goto done;
} else if (type == NFNL_MSG_BATCH_END) {
@@ -333,6 +380,7 @@ replay:
* original skb.
*/
if (err == -EAGAIN) {
+ nfnl_err_reset(&err_list);
ss->abort(skb);
nfnl_unlock(subsys_id);
kfree_skb(nskb);
@@ -341,11 +389,24 @@ replay:
}
ack:
if (nlh->nlmsg_flags & NLM_F_ACK || err) {
+ /* Errors are delivered once the full batch has been
+ * processed, this avoids that the same error is
+ * reported several times when replaying the batch.
+ */
+ if (nfnl_err_add(&err_list, nlh, err) < 0) {
+ /* We failed to enqueue an error, reset the
+ * list of errors and send OOM to userspace
+ * pointing to the batch header.
+ */
+ nfnl_err_reset(&err_list);
+ netlink_ack(skb, nlmsg_hdr(oskb), -ENOMEM);
+ success = false;
+ goto done;
+ }
/* We don't stop processing the batch on errors, thus,
* userspace gets all the errors that the batch
* triggers.
*/
- netlink_ack(skb, nlh, err);
if (err)
success = false;
}
@@ -361,6 +422,7 @@ done:
else
ss->abort(skb);
+ nfnl_err_deliver(&err_list, oskb);
nfnl_unlock(subsys_id);
kfree_skb(nskb);
}
--
1.7.10.4
^ permalink raw reply related
* [PATCH 3/5] rhashtable: fix lockdep splat in rhashtable_destroy()
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>
No need for rht_dereference() from rhashtable_destroy() since the
existing callers don't hold the mutex when invoking this function
from:
1) Netlink, this is called in case of memory allocation errors in the
initialization path, no nl_sk_hash_lock is held.
2) Netfilter, this is called from the rcu callback, no nfnl_lock is
held either.
I think it's reasonable to assume that the caller has to make sure
that no hash resizing may happen before releasing the bucket array.
Therefore, the caller should be responsible for releasing this in a
safe way, document this to make people aware of it.
This resolves a rcu lockdep splat in nft_hash:
===============================
[ INFO: suspicious RCU usage. ]
3.16.0+ #178 Not tainted
-------------------------------
lib/rhashtable.c:596 suspicious rcu_dereference_protected() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1
1 lock held by ksoftirqd/2/18:
#0: (rcu_callback){......}, at: [<ffffffff810918fd>] rcu_process_callbacks+0x27e/0x4c7
stack backtrace:
CPU: 2 PID: 18 Comm: ksoftirqd/2 Not tainted 3.16.0+ #178
Hardware name: LENOVO 23259H1/23259H1, BIOS G2ET32WW (1.12 ) 05/30/2012
0000000000000001 ffff88011706bb68 ffffffff8143debc 0000000000000000
ffff880117062610 ffff88011706bb98 ffffffff81077515 ffff8800ca041a50
0000000000000004 ffff8800ca386480 ffff8800ca041a00 ffff88011706bbb8
Call Trace:
[<ffffffff8143debc>] dump_stack+0x4e/0x68
[<ffffffff81077515>] lockdep_rcu_suspicious+0xfa/0x103
[<ffffffff81228b1b>] rhashtable_destroy+0x46/0x52
[<ffffffffa06f21a7>] nft_hash_destroy+0x73/0x82 [nft_hash]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Thomas Graf <tgraf@suug.ch>
---
lib/rhashtable.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index a2c7881..fc0dd8e 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -589,13 +589,13 @@ EXPORT_SYMBOL_GPL(rhashtable_init);
* rhashtable_destroy - destroy hash table
* @ht: the hash table to destroy
*
- * Frees the bucket array.
+ * Frees the bucket array. This function is not rcu safe, therefore the caller
+ * has to make sure that no resizing may happen by unpublishing the hashtable
+ * and waiting for the quiescent cycle before releasing the bucket array.
*/
void rhashtable_destroy(const struct rhashtable *ht)
{
- const struct bucket_table *tbl = rht_dereference(ht->tbl, ht);
-
- bucket_table_free(tbl);
+ bucket_table_free(ht->tbl);
}
EXPORT_SYMBOL_GPL(rhashtable_destroy);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/5] netfilter: nft_hash: no need for rcu in the hash set destroy path
From: Pablo Neira Ayuso @ 2014-09-23 9:24 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1411464288-18069-1-git-send-email-pablo@netfilter.org>
The sets are released from the rcu callback, after the rule is removed
from the chain list, which implies that nfnetlink cannot update the
hashes (thus, no resizing may occur) and no packets are walking on the
set anymore.
This resolves a lockdep splat in the nft_hash_destroy() path since the
nfnl mutex is not held there.
===============================
[ INFO: suspicious RCU usage. ]
3.16.0-rc2+ #168 Not tainted
-------------------------------
net/netfilter/nft_hash.c:362 suspicious rcu_dereference_protected() usage!
other info that might help us debug this:
rcu_scheduler_active = 1, debug_locks = 1
1 lock held by ksoftirqd/0/3:
#0: (rcu_callback){......}, at: [<ffffffff81096393>] rcu_process_callbacks+0x27e/0x4c7
stack backtrace:
CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 3.16.0-rc2+ #168
Hardware name: LENOVO 23259H1/23259H1, BIOS G2ET32WW (1.12 ) 05/30/2012
0000000000000001 ffff88011769bb98 ffffffff8142c922 0000000000000006
ffff880117694090 ffff88011769bbc8 ffffffff8107c3ff ffff8800cba52400
ffff8800c476bea8 ffff8800c476bea8 ffff8800cba52400 ffff88011769bc08
Call Trace:
[<ffffffff8142c922>] dump_stack+0x4e/0x68
[<ffffffff8107c3ff>] lockdep_rcu_suspicious+0xfa/0x103
[<ffffffffa079931e>] nft_hash_destroy+0x50/0x137 [nft_hash]
[<ffffffffa078cd57>] nft_set_destroy+0x11/0x2a [nf_tables]
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Acked-by: Thomas Graf <tgraf@suug.ch>
---
net/netfilter/nft_hash.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nft_hash.c b/net/netfilter/nft_hash.c
index 28fb8f3..8892b7b 100644
--- a/net/netfilter/nft_hash.c
+++ b/net/netfilter/nft_hash.c
@@ -180,15 +180,17 @@ static int nft_hash_init(const struct nft_set *set,
static void nft_hash_destroy(const struct nft_set *set)
{
const struct rhashtable *priv = nft_set_priv(set);
- const struct bucket_table *tbl;
+ const struct bucket_table *tbl = priv->tbl;
struct nft_hash_elem *he, *next;
unsigned int i;
- tbl = rht_dereference(priv->tbl, priv);
- for (i = 0; i < tbl->size; i++)
- rht_for_each_entry_safe(he, next, tbl->buckets[i], priv, node)
+ for (i = 0; i < tbl->size; i++) {
+ for (he = rht_entry(tbl->buckets[i], struct nft_hash_elem, node);
+ he != NULL; he = next) {
+ next = rht_entry(he->node.next, struct nft_hash_elem, node);
nft_hash_elem_destroy(set, he);
-
+ }
+ }
rhashtable_destroy(priv);
}
--
1.7.10.4
^ permalink raw reply related
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Thomas Graf @ 2014-09-23 9:18 UTC (permalink / raw)
To: Tom Herbert
Cc: ryazanov.s.a-Re5JQEeQqe8AvxtiuMwx3w, Jason Wang, John Fastabend,
Neil Jerram, Eric Dumazet, Andy Gospodarek,
dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Felix Fietkau,
Florian Fainelli, ronye-VPRAkNaXOzVWk0Htik3J/w, Jeff Kirsher,
Or Gerlitz, Ben Hutchings, Lennert Buytenhek, Alexander Duyck,
Jiri Pirko, simon.horman-wFxRvT7yatFl57MIdRCFDg, Roopa Prabhu,
Jamal Hadi Salim, aviadr-VPRAkNaXOzVWk0Htik3J/w, Nicolas Dichtel,
Vladislav Yasevich, Neil Horman,
"netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <netdev@
In-Reply-To: <CA+mtBx9ZVQ5r5Hzy9-uEnk+iu+HKkOP4+VANC06Xf8VvTxktwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/22/14 at 03:40pm, Tom Herbert wrote:
> On Mon, Sep 22, 2014 at 3:17 PM, Thomas Graf <tgraf-G/eBtMaohhA@public.gmane.org> wrote:
> > What makes stateful offload interesting to me is that the final
> > desintation of a packet is known at RX and can be redirected to a
> > queue or VF. This allows to build packet batches on shared pages
> > while preserving the securiy model.
To put this in other words: It is equivalent to applying the snabbswitch
+ vhost-user principle to the kernel but with encap support. The SR-IOV
case would be a further optimization of that.
^ permalink raw reply
* Re: [PATCH net-next v2 2/5] net: tcp: add flag for ca to indicate that ECN is required
From: Daniel Borkmann @ 2014-09-23 9:17 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Florian Westphal, davem, hagen, lars, eric.dumazet, fontana,
hannes, glenn.judd, netdev
In-Reply-To: <20140922092606.10977e48@urahara>
Hi Stephen,
On 09/22/2014 06:26 PM, Stephen Hemminger wrote:
> On Sat, 20 Sep 2014 23:29:19 +0200
> Florian Westphal <fw@strlen.de> wrote:
>
>> +static inline void
>> +TCP_ECN_create_request(struct request_sock *req, const struct sk_buff *skb,
>> + const struct sock *listen_sk)
>
> Maybe good time to switch to lower case here for function name.
thanks for your feedback and review, much appreciated! I think
it makes sense and we would send a follow-up clean-up patch that
replaces TCP_ECN_* with tcp_ecn_* in this and other function names.
Thanks,
Daniel, Florian
^ 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