* [gcv v4 07/38] net: Replace __get_cpu_var uses
From: Christoph Lameter @ 2013-09-03 19:12 UTC (permalink / raw)
To: Tejun Heo
Cc: akpm, David S. Miller, netdev, linux-arch, Steven Rostedt,
linux-kernel, Ingo Molnar, Peter Zijlstra
In-Reply-To: <20130903190014.794420260@linux.com>
__get_cpu_var() is used for multiple purposes in the kernel source. One of them is
address calculation via the form &__get_cpu_var(x). This calculates the address for
the instance of the percpu variable of the current processor based on an offset.
Other use cases are for storing and retrieving data from the current processors percpu area.
__get_cpu_var() can be used as an lvalue when writing data or on the right side of an assignment.
__get_cpu_var() is defined as :
#define __get_cpu_var(var) (*this_cpu_ptr(&(var)))
__get_cpu_var() always only does an address determination. However, store and retrieve operations
could use a segment prefix (or global register on other platforms) to avoid the address calculation.
this_cpu_write() and this_cpu_read() can directly take an offset into a percpu area and use
optimized assembly code to read and write per cpu variables.
This patch converts __get_cpu_var into either an explicit address calculation using this_cpu_ptr()
or into a use of this_cpu operations that use the offset. Thereby address calculations are avoided
and less registers are used when code is generated.
At the end of the patch set all uses of __get_cpu_var have been removed so the macro is removed too.
The patch set includes passes over all arches as well. Once these operations are used throughout then
specialized macros can be defined in non -x86 arches as well in order to optimize per cpu access by
f.e. using a global register that may be set to the per cpu base.
Transformations done to __get_cpu_var()
1. Determine the address of the percpu instance of the current processor.
DEFINE_PER_CPU(int, y);
int *x = &__get_cpu_var(y);
Converts to
int *x = this_cpu_ptr(&y);
2. Same as #1 but this time an array structure is involved.
DEFINE_PER_CPU(int, y[20]);
int *x = __get_cpu_var(y);
Converts to
int *x = this_cpu_ptr(y);
3. Retrieve the content of the current processors instance of a per cpu variable.
DEFINE_PER_CPU(int, y);
int x = __get_cpu_var(y)
Converts to
int x = __this_cpu_read(y);
4. Retrieve the content of a percpu struct
DEFINE_PER_CPU(struct mystruct, y);
struct mystruct x = __get_cpu_var(y);
Converts to
memcpy(&x, this_cpu_ptr(&y), sizeof(x));
5. Assignment to a per cpu variable
DEFINE_PER_CPU(int, y)
__get_cpu_var(y) = x;
Converts to
this_cpu_write(y, x);
6. Increment/Decrement etc of a per cpu variable
DEFINE_PER_CPU(int, y);
__get_cpu_var(y)++
Converts to
this_cpu_inc(y)
Signed-off-by: Christoph Lameter <cl@linux.com>
Index: linux/net/core/dev.c
===================================================================
--- linux.orig/net/core/dev.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/dev.c 2013-08-26 14:18:37.214005168 -0500
@@ -2129,7 +2129,7 @@ static inline void __netif_reschedule(st
unsigned long flags;
local_irq_save(flags);
- sd = &__get_cpu_var(softnet_data);
+ sd = this_cpu_ptr(&softnet_data);
q->next_sched = NULL;
*sd->output_queue_tailp = q;
sd->output_queue_tailp = &q->next_sched;
@@ -2151,7 +2151,7 @@ void dev_kfree_skb_irq(struct sk_buff *s
unsigned long flags;
local_irq_save(flags);
- sd = &__get_cpu_var(softnet_data);
+ sd = this_cpu_ptr(&softnet_data);
skb->next = sd->completion_queue;
sd->completion_queue = skb;
raise_softirq_irqoff(NET_TX_SOFTIRQ);
@@ -3111,7 +3111,7 @@ static void rps_trigger_softirq(void *da
static int rps_ipi_queued(struct softnet_data *sd)
{
#ifdef CONFIG_RPS
- struct softnet_data *mysd = &__get_cpu_var(softnet_data);
+ struct softnet_data *mysd = this_cpu_ptr(&softnet_data);
if (sd != mysd) {
sd->rps_ipi_next = mysd->rps_ipi_list;
@@ -3138,7 +3138,7 @@ static bool skb_flow_limit(struct sk_buf
if (qlen < (netdev_max_backlog >> 1))
return false;
- sd = &__get_cpu_var(softnet_data);
+ sd = this_cpu_ptr(&softnet_data);
rcu_read_lock();
fl = rcu_dereference(sd->flow_limit);
@@ -3280,7 +3280,7 @@ EXPORT_SYMBOL(netif_rx_ni);
static void net_tx_action(struct softirq_action *h)
{
- struct softnet_data *sd = &__get_cpu_var(softnet_data);
+ struct softnet_data *sd = this_cpu_ptr(&softnet_data);
if (sd->completion_queue) {
struct sk_buff *clist;
@@ -3700,7 +3700,7 @@ EXPORT_SYMBOL(netif_receive_skb);
static void flush_backlog(void *arg)
{
struct net_device *dev = arg;
- struct softnet_data *sd = &__get_cpu_var(softnet_data);
+ struct softnet_data *sd = this_cpu_ptr(&softnet_data);
struct sk_buff *skb, *tmp;
rps_lock(sd);
@@ -4146,7 +4146,7 @@ void __napi_schedule(struct napi_struct
unsigned long flags;
local_irq_save(flags);
- ____napi_schedule(&__get_cpu_var(softnet_data), n);
+ ____napi_schedule(this_cpu_ptr(&softnet_data), n);
local_irq_restore(flags);
}
EXPORT_SYMBOL(__napi_schedule);
@@ -4274,7 +4274,7 @@ EXPORT_SYMBOL(netif_napi_del);
static void net_rx_action(struct softirq_action *h)
{
- struct softnet_data *sd = &__get_cpu_var(softnet_data);
+ struct softnet_data *sd = this_cpu_ptr(&softnet_data);
unsigned long time_limit = jiffies + 2;
int budget = netdev_budget;
void *have;
Index: linux/net/core/drop_monitor.c
===================================================================
--- linux.orig/net/core/drop_monitor.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/drop_monitor.c 2013-08-26 14:18:37.218005126 -0500
@@ -142,7 +142,7 @@ static void trace_drop_common(struct sk_
unsigned long flags;
local_irq_save(flags);
- data = &__get_cpu_var(dm_cpu_data);
+ data = this_cpu_ptr(&dm_cpu_data);
spin_lock(&data->lock);
dskb = data->skb;
Index: linux/net/core/skbuff.c
===================================================================
--- linux.orig/net/core/skbuff.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/core/skbuff.c 2013-08-26 14:18:37.218005126 -0500
@@ -371,7 +371,7 @@ static void *__netdev_alloc_frag(unsigne
unsigned long flags;
local_irq_save(flags);
- nc = &__get_cpu_var(netdev_alloc_cache);
+ nc = this_cpu_ptr(&netdev_alloc_cache);
if (unlikely(!nc->frag.page)) {
refill:
for (order = NETDEV_FRAG_PAGE_MAX_ORDER; ;) {
Index: linux/net/ipv4/syncookies.c
===================================================================
--- linux.orig/net/ipv4/syncookies.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv4/syncookies.c 2013-08-26 14:18:37.210005210 -0500
@@ -44,7 +44,7 @@ static DEFINE_PER_CPU(__u32 [16 + 5 + SH
static u32 cookie_hash(__be32 saddr, __be32 daddr, __be16 sport, __be16 dport,
u32 count, int c)
{
- __u32 *tmp = __get_cpu_var(ipv4_cookie_scratch);
+ __u32 *tmp = this_cpu_ptr(ipv4_cookie_scratch);
memcpy(tmp + 4, syncookie_secret[c], sizeof(syncookie_secret[c]));
tmp[0] = (__force u32)saddr;
Index: linux/net/ipv4/tcp_output.c
===================================================================
--- linux.orig/net/ipv4/tcp_output.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv4/tcp_output.c 2013-08-26 14:18:37.218005126 -0500
@@ -810,7 +810,7 @@ void tcp_wfree(struct sk_buff *skb)
/* queue this socket to tasklet queue */
local_irq_save(flags);
- tsq = &__get_cpu_var(tsq_tasklet);
+ tsq = this_cpu_ptr(&tsq_tasklet);
list_add(&tp->tsq_node, &tsq->head);
tasklet_schedule(&tsq->tasklet);
local_irq_restore(flags);
Index: linux/net/ipv6/syncookies.c
===================================================================
--- linux.orig/net/ipv6/syncookies.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/ipv6/syncookies.c 2013-08-26 14:18:37.214005168 -0500
@@ -66,7 +66,7 @@ static DEFINE_PER_CPU(__u32 [16 + 5 + SH
static u32 cookie_hash(const struct in6_addr *saddr, const struct in6_addr *daddr,
__be16 sport, __be16 dport, u32 count, int c)
{
- __u32 *tmp = __get_cpu_var(ipv6_cookie_scratch);
+ __u32 *tmp = this_cpu_ptr(ipv6_cookie_scratch);
/*
* we have 320 bits of information to hash, copy in the remaining
Index: linux/net/rds/ib_rdma.c
===================================================================
--- linux.orig/net/rds/ib_rdma.c 2013-08-26 14:16:59.000000000 -0500
+++ linux/net/rds/ib_rdma.c 2013-08-26 14:18:37.218005126 -0500
@@ -267,7 +267,7 @@ static inline struct rds_ib_mr *rds_ib_r
unsigned long *flag;
preempt_disable();
- flag = &__get_cpu_var(clean_list_grace);
+ flag = this_cpu_ptr(&clean_list_grace);
set_bit(CLEAN_LIST_BUSY_BIT, flag);
ret = llist_del_first(&pool->clean_list);
if (ret)
Index: linux/include/net/netfilter/nf_conntrack.h
===================================================================
--- linux.orig/include/net/netfilter/nf_conntrack.h 2013-08-26 14:23:46.000000000 -0500
+++ linux/include/net/netfilter/nf_conntrack.h 2013-08-26 14:24:17.190395755 -0500
@@ -243,7 +243,7 @@ extern s16 (*nf_ct_nat_offset)(const str
DECLARE_PER_CPU(struct nf_conn, nf_conntrack_untracked);
static inline struct nf_conn *nf_ct_untracked_get(void)
{
- return &__raw_get_cpu_var(nf_conntrack_untracked);
+ return __this_cpu_ptr(&nf_conntrack_untracked);
}
extern void nf_ct_untracked_status_or(unsigned long bits);
^ permalink raw reply
* Re: [PATCH][net-next] gianfar: Fix reported number of sent bytes to BQL
From: Florian Fainelli @ 2013-09-03 18:59 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, David S. Miller, Eric Dumazet
In-Reply-To: <1377864075-19491-1-git-send-email-claudiu.manoil@freescale.com>
Hello Claudiu,
2013/8/30 Claudiu Manoil <claudiu.manoil@freescale.com>:
> Fix the amount of sent bytes reported to BQL by reporting the
> number of bytes on wire in the xmit routine, and recording that
> value for each skb in order to be correctly confirmed on Tx
> confirmation cleanup.
>
> Reporting skb->len to BQL just before exiting xmit is not correct
> due to possible insertions of TOE block and alignment bytes in the
> skb->data, which are being stripped off by the controller before
> transmission on wire. This led to mismatch of (incorrectly)
> reported bytes to BQL b/w xmit and Tx confirmation, resulting in
> Tx timeout firing, for the h/w tx timestamping acceleration case.
>
> There's no easy way to obtain the number of bytes on wire in the Tx
> confirmation routine, so skb->cb is used to convey that information
> from xmit to Tx confirmation, for now (as proposed by Eric). Revived
> the currently unused GFAR_CB() construct for that purpose.
I do not see much difference between what this patch does and what the
current net-next drivers does. If you need to correctly account for
this, it seems to me like you should move the stats/bytes_sent
computation below this line:
if (unlikely(do_tstamp)) {
skb_push(skb, GMAC_TXPAL_LEN);
memset(skb->data, 0, GMAC_TXPAL_LEN);
}
to account for the SKB length update?
>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 18 +++++++++++-------
> drivers/net/ethernet/freescale/gianfar.h | 2 +-
> 2 files changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index b2c91dc..c4eaade 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2092,7 +2092,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> int do_tstamp, do_csum, do_vlan;
> u32 bufaddr;
> unsigned long flags;
> - unsigned int nr_frags, nr_txbds, length, fcb_len = 0;
> + unsigned int nr_frags, nr_txbds, bytes_sent, fcb_len = 0;
>
> rq = skb->queue_mapping;
> tx_queue = priv->tx_queue[rq];
> @@ -2147,7 +2147,10 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> }
>
> /* Update transmit stats */
> - tx_queue->stats.tx_bytes += skb->len;
> + bytes_sent = skb->len;
> + tx_queue->stats.tx_bytes += bytes_sent;
> + /* keep Tx bytes on wire for BQL accounting */
> + GFAR_CB(skb)->bytes_sent = bytes_sent;
> tx_queue->stats.tx_packets++;
>
> txbdp = txbdp_start = tx_queue->cur_tx;
> @@ -2167,12 +2170,13 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> } else {
> /* Place the fragment addresses and lengths into the TxBDs */
> for (i = 0; i < nr_frags; i++) {
> + unsigned int frag_len;
> /* Point at the next BD, wrapping as needed */
> txbdp = next_txbd(txbdp, base, tx_queue->tx_ring_size);
>
> - length = skb_shinfo(skb)->frags[i].size;
> + frag_len = skb_shinfo(skb)->frags[i].size;
>
> - lstatus = txbdp->lstatus | length |
> + lstatus = txbdp->lstatus | frag_len |
> BD_LFLAG(TXBD_READY);
>
> /* Handle the last BD specially */
> @@ -2182,7 +2186,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> bufaddr = skb_frag_dma_map(priv->dev,
> &skb_shinfo(skb)->frags[i],
> 0,
> - length,
> + frag_len,
> DMA_TO_DEVICE);
>
> /* set the TxBD length and buffer pointer */
> @@ -2250,7 +2254,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> lstatus |= BD_LFLAG(TXBD_CRC | TXBD_READY) | skb_headlen(skb);
> }
>
> - netdev_tx_sent_queue(txq, skb->len);
> + netdev_tx_sent_queue(txq, bytes_sent);
>
> /* We can work in parallel with gfar_clean_tx_ring(), except
> * when modifying num_txbdfree. Note that we didn't grab the lock
> @@ -2570,7 +2574,7 @@ static void gfar_clean_tx_ring(struct gfar_priv_tx_q *tx_queue)
> bdp = next_txbd(bdp, base, tx_ring_size);
> }
>
> - bytes_sent += skb->len;
> + bytes_sent += GFAR_CB(skb)->bytes_sent;
>
> dev_kfree_skb_any(skb);
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
> index 46f56f3..04112b9 100644
> --- a/drivers/net/ethernet/freescale/gianfar.h
> +++ b/drivers/net/ethernet/freescale/gianfar.h
> @@ -575,7 +575,7 @@ struct rxfcb {
> };
>
> struct gianfar_skb_cb {
> - int alignamount;
> + unsigned int bytes_sent; /* bytes-on-wire (i.e. no FCB) */
> };
>
> #define GFAR_CB(skb) ((struct gianfar_skb_cb *)((skb)->cb))
> --
> 1.7.11.7
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
From: Ben Hutchings @ 2013-09-03 18:53 UTC (permalink / raw)
To: David Daney; +Cc: David Miller, David Daney, netdev
In-Reply-To: <52261D12.5000009@gmail.com>
On Tue, Sep 03, 2013 at 10:32:02AM -0700, David Daney wrote:
> On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> >bcm87xx currently isn't auto-loaded if built as a module.
> >
> >Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> >---
> >Compile-tested only.
>
> Then how do you know that it does anything sensible?
This is generally required in modular PHY drivers. I was hoping you'd
be able to say whether that it's useful or necessary for this one.
> Other than that, it seems plausible.
>
> David Daney
Thanks.
Ben.
> >
> >Ben.
> >
> > drivers/net/phy/bcm87xx.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> >diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> >index 7997895..f5b44ff 100644
> >--- a/drivers/net/phy/bcm87xx.c
> >+++ b/drivers/net/phy/bcm87xx.c
> >@@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
> > }
> > module_exit(bcm87xx_exit);
> >
> >+static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
> >+ { PHY_ID_BCM8706, 0xffffffff },
> >+ { PHY_ID_BCM8727, 0xffffffff },
> >+ { }
> >+};
> >+
> > MODULE_LICENSE("GPL");
> >+MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
> >
>
--
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
- Albert Camus
^ permalink raw reply
* Re: [PATCH net-next 8/8] net: ipv6: mld: document force_mld_version in ip-sysctl.txt
From: Hannes Frederic Sowa @ 2013-09-03 18:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-9-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:38AM +0200, Daniel Borkmann wrote:
> Document force_mld_version parameter in ip-sysctl.txt.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Daniel, great series. It is now easier to follow the rfc and check the code
for correctness!
Thanks a lot,
Hannes
^ permalink raw reply
* Re: [PATCH net-next 7/8] net: ipv6: mld: introduce mld_{gq,ifc,dad}_stop_timer functions
From: Hannes Frederic Sowa @ 2013-09-03 18:51 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-8-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:37AM +0200, Daniel Borkmann wrote:
> We already have mld_{gq,ifc,dad}_start_timer() functions, so introduce
> mld_{gq,ifc,dad}_stop_timer() functions to reduce code size and make it
> more readable.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH net-next 6/8] net: ipv6: mld: refactor query processing into v1/v2 functions
From: Hannes Frederic Sowa @ 2013-09-03 18:49 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-7-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:36AM +0200, Daniel Borkmann wrote:
> Make igmp6_event_query() a bit easier to read by refactoring code
> parts into mld_process_v1() and mld_process_v2().
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH net-next 5/8] net: ipv6: mld: similarly to MLDv2 have min max_delay of 1
From: Hannes Frederic Sowa @ 2013-09-03 18:38 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-6-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:35AM +0200, Daniel Borkmann wrote:
> Similarly as we do in MLDv2 queries, set a forged MLDv1 query with
> 0 ms mld_maxdelay to minimum timer shot time of 1 jiffies. This is
> eventually done in igmp6_group_queried() anyway, so we can simplify
> a check there.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Hannes Frederic Sowa @ 2013-09-03 18:30 UTC (permalink / raw)
To: Thomas Graf; +Cc: Stephen Warren, Eric Dumazet, davem, netdev, Fabio Estevam
In-Reply-To: <20130903182348.GB4166@casper.infradead.org>
On Tue, Sep 03, 2013 at 07:23:48PM +0100, Thomas Graf wrote:
> On 09/03/13 at 12:03pm, Stephen Warren wrote:
> > On 09/03/2013 11:51 AM, Eric Dumazet wrote:
> > > On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
> > >
> > >> I don't see any extra messages from PROVE_LOCKING related to networking.
> > >> There is a single extra message from inside the audio driver, but that's
> > >> not networking-related at all.
> > >
> > > LOCKDEP is automatically disabled at first splat.
> > >
> > > Please try a kernel without audio driver ;)
> >
> > Ah, OK. Now I do see something from ipv6:
> >
> > > [ 25.327622]
> > > [ 25.329142] =============================================
> > > [ 25.334533] [ INFO: possible recursive locking detected ]
> > > [ 25.339927] 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17 Not tainted
> > > [ 25.346705] ---------------------------------------------
> > > [ 25.352095] login/704 is trying to acquire lock:
> > > [ 25.356705] (&ndev->lock){++--..}, at: [<c049be24>] ipv6_chk_mcast_addr+0x5c/0x200
> > > [ 25.364405]
> > > [ 25.364405] but task is already holding lock:
> > > [ 25.370230] (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
>
> Real deadlock, we should not hold idev->lock for ndisc_send_rs(), we
> should drop the lock beforehand I guess. We also don't hold idev->lock
> if we send out the RS via addrconf_dad_completed().
Yes, this already happend by the series to implement ipv6 for vxlan:
caf92bc4007036cfac8ee06c845fdfe6496e4fb3 ("ipv6: do not call ndisc_send_rs()
with write lock").
> I'm confused why lockdep would only trigger after my patch, the
> deadlock is unrelated.
Reordering by the compiler in the short-curcuit evaluation in
ip6_finish_output2 maybe (sk_mc_loop)?
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Thomas Graf @ 2013-09-03 18:23 UTC (permalink / raw)
To: Stephen Warren; +Cc: Eric Dumazet, davem, netdev, Fabio Estevam
In-Reply-To: <52262462.80308@wwwdotorg.org>
On 09/03/13 at 12:03pm, Stephen Warren wrote:
> On 09/03/2013 11:51 AM, Eric Dumazet wrote:
> > On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
> >
> >> I don't see any extra messages from PROVE_LOCKING related to networking.
> >> There is a single extra message from inside the audio driver, but that's
> >> not networking-related at all.
> >
> > LOCKDEP is automatically disabled at first splat.
> >
> > Please try a kernel without audio driver ;)
>
> Ah, OK. Now I do see something from ipv6:
>
> > [ 25.327622]
> > [ 25.329142] =============================================
> > [ 25.334533] [ INFO: possible recursive locking detected ]
> > [ 25.339927] 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17 Not tainted
> > [ 25.346705] ---------------------------------------------
> > [ 25.352095] login/704 is trying to acquire lock:
> > [ 25.356705] (&ndev->lock){++--..}, at: [<c049be24>] ipv6_chk_mcast_addr+0x5c/0x200
> > [ 25.364405]
> > [ 25.364405] but task is already holding lock:
> > [ 25.370230] (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
Real deadlock, we should not hold idev->lock for ndisc_send_rs(), we
should drop the lock beforehand I guess. We also don't hold idev->lock
if we send out the RS via addrconf_dad_completed().
I'm confused why lockdep would only trigger after my patch, the
deadlock is unrelated.
^ permalink raw reply
* Re: [PATCH net-next 4/8] net: ipv6: mld: implement RFC3810 MLDv2 mode only
From: Hannes Frederic Sowa @ 2013-09-03 18:12 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-5-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:34AM +0200, Daniel Borkmann wrote:
> RFC3810, 10. Security Considerations says under subsection 10.1.
> Query Message:
>
> A forged Version 1 Query message will put MLDv2 listeners on that
> link in MLDv1 Host Compatibility Mode. This scenario can be avoided
> by providing MLDv2 hosts with a configuration option to ignore
> Version 1 messages completely.
>
> Hence, implement a MLDv2-only mode that will ignore MLDv1 traffic:
>
> echo 2 > /proc/sys/net/ipv6/conf/ethX/force_mld_version
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH net-next 3/8] net: ipv6: mld: get rid of MLDV2_MRC and simplify calculation
From: Hannes Frederic Sowa @ 2013-09-03 18:09 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-4-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:33AM +0200, Daniel Borkmann wrote:
> Get rid of MLDV2_MRC and use our new macros for mantisse and
> exponent to calculate Maximum Response Delay out of the Maximum
> Response Code.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 18:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Thomas Graf, davem, netdev, Fabio Estevam
In-Reply-To: <1378230697.7360.38.camel@edumazet-glaptop>
On 09/03/2013 11:51 AM, Eric Dumazet wrote:
> On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
>
>> I don't see any extra messages from PROVE_LOCKING related to networking.
>> There is a single extra message from inside the audio driver, but that's
>> not networking-related at all.
>
> LOCKDEP is automatically disabled at first splat.
>
> Please try a kernel without audio driver ;)
Ah, OK. Now I do see something from ipv6:
> [ 25.327622]
> [ 25.329142] =============================================
> [ 25.334533] [ INFO: possible recursive locking detected ]
> [ 25.339927] 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17 Not tainted
> [ 25.346705] ---------------------------------------------
> [ 25.352095] login/704 is trying to acquire lock:
> [ 25.356705] (&ndev->lock){++--..}, at: [<c049be24>] ipv6_chk_mcast_addr+0x5c/0x200
> [ 25.364405]
> [ 25.364405] but task is already holding lock:
> [ 25.370230] (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
> [ 25.377741]
> [ 25.377741] other info that might help us debug this:
> [ 25.384261] Possible unsafe locking scenario:
> [ 25.384261]
> [ 25.390174] CPU0
> [ 25.392614] ----
> [ 25.395053] lock(&ndev->lock);
> [ 25.398286] lock(&ndev->lock);
> [ 25.401518]
> [ 25.401518] *** DEADLOCK ***
> [ 25.401518]
> [ 25.407433] May be due to missing lock nesting notation
> [ 25.407433]
> [ 25.414214] 4 locks held by login/704:
> [ 25.417955] #0: (((&ndev->rs_timer))){+.-...}, at: [<c002f8a8>] call_timer_fn+0x0/0xe4
> [ 25.426076] #1: (&ndev->lock){++--..}, at: [<c0480eb4>] addrconf_rs_timer+0x18/0x134
> [ 25.434018] #2: (rcu_read_lock){.+.+..}, at: [<c048e104>] ndisc_send_skb+0x58/0x3dc
> [ 25.441876] #3: (rcu_read_lock){.+.+..}, at: [<c049bdc8>] ipv6_chk_mcast_addr+0x0/0x200
> [ 25.450079]
> [ 25.450079] stack backtrace:
> [ 25.454436] CPU: 0 PID: 704 Comm: login Not tainted 3.11.0-rc7-next-20130830-00024-g209b4d8-dirty #17
> [ 25.463674] [<c0016958>] (unwind_backtrace+0x0/0x138) from [<c00125fc>] (show_stack+0x10/0x14)
> [ 25.472297] [<c00125fc>] (show_stack+0x10/0x14) from [<c0567808>] (dump_stack+0x80/0xc4)
> [ 25.480394] [<c0567808>] (dump_stack+0x80/0xc4) from [<c0074af8>] (validate_chain.isra.24+0x74c/0x8c8)
> [ 25.489696] [<c0074af8>] (validate_chain.isra.24+0x74c/0x8c8) from [<c00757b0>] (__lock_acquire+0x3bc/0xa80)
> [ 25.499518] [<c00757b0>] (__lock_acquire+0x3bc/0xa80) from [<c007632c>] (lock_acquire+0x60/0x74)
> [ 25.508305] [<c007632c>] (lock_acquire+0x60/0x74) from [<c056e998>] (_raw_read_lock_bh+0x4c/0x5c)
> [ 25.517175] [<c056e998>] (_raw_read_lock_bh+0x4c/0x5c) from [<c049be24>] (ipv6_chk_mcast_addr+0x5c/0x200)
> [ 25.526740] [<c049be24>] (ipv6_chk_mcast_addr+0x5c/0x200) from [<c04799ac>] (ip6_finish_output2+0x298/0x62c)
> [ 25.536563] [<c04799ac>] (ip6_finish_output2+0x298/0x62c) from [<c048e2b0>] (ndisc_send_skb+0x204/0x3dc)
> [ 25.546037] [<c048e2b0>] (ndisc_send_skb+0x204/0x3dc) from [<c0480fa4>] (addrconf_rs_timer+0x108/0x134)
> [ 25.555425] [<c0480fa4>] (addrconf_rs_timer+0x108/0x134) from [<c002f914>] (call_timer_fn+0x6c/0xe4)
> [ 25.564553] [<c002f914>] (call_timer_fn+0x6c/0xe4) from [<c0030220>] (run_timer_softirq+0x180/0x22c)
> [ 25.573691] [<c0030220>] (run_timer_softirq+0x180/0x22c) from [<c002a210>] (__do_softirq+0x100/0x1fc)
> [ 25.582905] [<c002a210>] (__do_softirq+0x100/0x1fc) from [<c002a6d8>] (irq_exit+0xa8/0xf4)
> [ 25.591166] [<c002a6d8>] (irq_exit+0xa8/0xf4) from [<c000f9b8>] (handle_IRQ+0x3c/0x94)
> [ 25.599078] [<c000f9b8>] (handle_IRQ+0x3c/0x94) from [<c0008700>] (gic_handle_irq+0x28/0x5c)
> [ 25.607511] [<c0008700>] (gic_handle_irq+0x28/0x5c) from [<c00132fc>] (__irq_usr+0x3c/0x60)
> [ 25.615855] Exception stack(0xee0d9fb0 to 0xee0d9ff8)
> [ 25.620901] 9fa0: 5c026e4c c9ce71d5 538e8b11 f2a43512
> [ 25.629072] 9fc0: 49f97e42 7a179ff1 9c755700 9b933273 8b7a2401 6d6222ee 7b8908fd 1df2f704
> [ 25.637243] 9fe0: 2664e738 bec3ef10 b6d440e9 b6d43d58 900f0030 ffffffff
^ permalink raw reply
* Re: VLAN HW accel, performance advantage?
From: Joakim Tjernlund @ 2013-09-03 18:01 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Ben Hutchings, netdev
In-Reply-To: <1378222150.7360.28.camel@edumazet-glaptop>
Eric Dumazet <eric.dumazet@gmail.com> wrote on 2013/09/03 17:29:10:
>
> On Tue, 2013-09-03 at 15:48 +0100, Ben Hutchings wrote:
>
> > GRO should be beneficial for bridges if you have bursty TCP streams.
It
> > is especially beneficial if you also have TSO on the TX side.
> >
> > Really you're going to have to try it and run benchmarks with your own
> > hardware.
>
> Note that ucc_geth does not have GRO support (it calls
> netif_receive_skb())
>
> It doesnt have RX checksuming either, so having to validate
> tcp checksums (before aggregating them in GRO stack) would hurt if
> packets only have to be forwarded.
Yes, the controller only have IP header checksum. Seems there is little to
gain with HW VLAN support for this controller, maybe when I got some spare
time
then. :)
Thanks guys
Jocke
^ permalink raw reply
* Re: [PATCH net-next 2/8] net: ipv6: mld: clean up MLD_V1_SEEN macro
From: Hannes Frederic Sowa @ 2013-09-03 17:55 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1378195178-21002-3-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:32AM +0200, Daniel Borkmann wrote:
> Replace the macro with a function to make it more readable. GCC will
> eventually decide whether to inline this or not (also, that's not
> fast-path anyway).
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH net-next 1/8] net: ipv6: mld: fix v1/v2 switchback timeout to rfc3810, 9.12.
From: Hannes Frederic Sowa @ 2013-09-03 17:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, David Stevens
In-Reply-To: <1378195178-21002-2-git-send-email-dborkman@redhat.com>
On Tue, Sep 03, 2013 at 09:59:31AM +0200, Daniel Borkmann wrote:
> i) RFC3810, 9.2. Query Interval [QI] says:
>
> The Query Interval variable denotes the interval between General
> Queries sent by the Querier. Default value: 125 seconds. [...]
>
> ii) RFC3810, 9.3. Query Response Interval [QRI] says:
>
> The Maximum Response Delay used to calculate the Maximum Response
> Code inserted into the periodic General Queries. Default value:
> 10000 (10 seconds) [...] The number of seconds represented by the
> [Query Response Interval] must be less than the [Query Interval].
>
> iii) RFC3810, 9.12. Older Version Querier Present Timeout [OVQPT] says:
>
> The Older Version Querier Present Timeout is the time-out for
> transitioning a host back to MLDv2 Host Compatibility Mode. When an
> MLDv1 query is received, MLDv2 hosts set their Older Version Querier
> Present Timer to [Older Version Querier Present Timeout].
>
> This value MUST be ([Robustness Variable] times (the [Query Interval]
> in the last Query received)) plus ([Query Response Interval]).
>
> Hence, on *default* the timeout results in:
>
> [RV] = 2, [QI] = 125sec, [QRI] = 10sec
> [OVQPT] = [RV] * [QI] + [QRI] = 260sec
>
> Having that said, we currently calculate [OVQPT] (here given as 'switchback'
> variable) as ...
>
> switchback = (idev->mc_qrv + 1) * max_delay
>
> RFC3810, 9.12. says "the [Query Interval] in the last Query received". In
> section "9.14. Configuring timers", it is said:
>
> This section is meant to provide advice to network administrators on
> how to tune these settings to their network. Ambitious router
> implementations might tune these settings dynamically based upon
> changing characteristics of the network. [...]
>
> iv) RFC38010, 9.14.2. Query Interval:
>
> The overall level of periodic MLD traffic is inversely proportional
> to the Query Interval. A longer Query Interval results in a lower
> overall level of MLD traffic. The value of the Query Interval MUST
> be equal to or greater than the Maximum Response Delay used to
> calculate the Maximum Response Code inserted in General Query
> messages.
>
> I assume that was why switchback is calculated as is (3 * max_delay), although
> this setting seems to be meant for routers only to configure their [QI]
> interval for non-default intervals. So usage here like this is clearly wrong.
>
> Concluding, the current behaviour in IPv6's multicast code is not conform
> to the RFC as switch back is calculated wrongly. That is, it has a too small
> value, so MLDv2 hosts switch back again to MLDv2 way too early, i.e. ~30secs
> instead of ~260secs on default.
>
> Hence, introduce necessary helper functions and fix this up properly as it
> should be.
>
> Introduced in 06da92283 ("[IPV6]: Add MLDv2 support."). Credits to Hannes
> Frederic Sowa who also had a hand in this as well. Also thanks to Hangbin Liu
> who did initial testing.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: David Stevens <dlstevens@us.ibm.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Eric Dumazet @ 2013-09-03 17:51 UTC (permalink / raw)
To: Stephen Warren; +Cc: Thomas Graf, davem, netdev, Fabio Estevam
In-Reply-To: <52261F68.70907@wwwdotorg.org>
On Tue, 2013-09-03 at 11:42 -0600, Stephen Warren wrote:
> I don't see any extra messages from PROVE_LOCKING related to networking.
> There is a single extra message from inside the audio driver, but that's
> not networking-related at all.
LOCKDEP is automatically disabled at first splat.
Please try a kernel without audio driver ;)
^ permalink raw reply
* Re: [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Eric Dumazet @ 2013-09-03 17:48 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, Jiri Benc
In-Reply-To: <1378229352-9779-1-git-send-email-dborkman@redhat.com>
On Tue, 2013-09-03 at 19:29 +0200, Daniel Borkmann wrote:
> In tcp_v6_do_rcv() code, when processing pkt options, we soley work
> on our skb clone opt_skb that we've created earlier before entering
> tcp_rcv_established() on our way. However, only in condition ...
>
> if (np->rxopt.bits.rxtclass)
> np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
>
> ... we work on skb itself. As we extract every other information out
> of opt_skb in ipv6_pktoptions path, this seems wrong, since skb can
> already be released by tcp_rcv_established() earlier on. When we try
> to access it in ipv6_hdr(), we will dereference freed skb.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/ipv6/tcp_ipv6.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 6e1649d..eeb4cb0 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1427,7 +1427,7 @@ ipv6_pktoptions:
> if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
> np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
> if (np->rxopt.bits.rxtclass)
> - np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
> + np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(opt_skb));
> if (ipv6_opt_accepted(sk, opt_skb)) {
> skb_set_owner_r(opt_skb, sk);
> opt_skb = xchg(&np->pktoptions, opt_skb);
Bug added in commit 4c507d2897bd9b
("net: implement IP_RECVTOS for IP_PKTOPTIONS")
CC Jiri
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 17:42 UTC (permalink / raw)
To: Thomas Graf, davem, netdev, Eric Dumazet, Fabio Estevam
In-Reply-To: <20130903172736.GB21729@order.stressinduktion.org>
On 09/03/2013 11:27 AM, Hannes Frederic Sowa wrote:
> On Tue, Sep 03, 2013 at 11:19:14AM -0600, Stephen Warren wrote:
>> On 09/03/2013 05:37 AM, Thomas Graf wrote:
>>> Allocating skbs when sending out neighbour discovery messages
>>> currently uses sock_alloc_send_skb() based on a per net namespace
>>> socket and thus share a socket wmem buffer space.
>>>
>>> If a netdevice is temporarily unable to transmit due to carrier
>>> loss or for other reasons, the queued up ndisc messages will cosnume
>>> all of the wmem space and will thus prevent from any more skbs to
>>> be allocated even for netdevices that are able to transmit packets.
>>>
>>> The number of neighbour discovery messages sent is very limited,
>>> use of alloc_skb() bypasses the socket wmem buffer size enforcement
>>> while the manual call to skb_set_owner_w() maintains the socket
>>> reference needed for the IPv6 output path.
>>>
>>> This patch has orginally been posted by Eric Dumazet in a modified
>>> form.
>>
>> Tested-by: Stephen Warren <swarren@nvidia.com>
>>
>> Although I do note something slightly odd:
>>
>> next-20130830 had an issue, and reverting V1 of this patch solved it.
>>
>> However, in next-20130903, if I revert the revert of V1 of this patch, I
>> don't see any issue; it appears that the problem was some interaction
>> between V1 of this patch and something else in next-20130830.
>>
>> Either way, this patch doesn't seem to introduce any issue when applied
>> on top of either next-20130830 with V1 reverted, or on top of
>> next-20130903, so it's fine.
>
> Could either of you run the v1 version of the patch with CONFIG_PROVE_LOCKING
> enabled? I also do think there is some side-effect we don't understand yet.
I don't see any extra messages from PROVE_LOCKING related to networking.
There is a single extra message from inside the audio driver, but that's
not networking-related at all.
^ permalink raw reply
* Re: [PATCH net-next] bcm87xx: Add MODULE_DEVICE_TABLE
From: David Daney @ 2013-09-03 17:32 UTC (permalink / raw)
To: Ben Hutchings, David Miller; +Cc: David Daney, netdev
In-Reply-To: <1378071184.25743.52.camel@deadeye.wl.decadent.org.uk>
On 09/01/2013 02:33 PM, Ben Hutchings wrote:
> bcm87xx currently isn't auto-loaded if built as a module.
>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> Compile-tested only.
Then how do you know that it does anything sensible?
Other than that, it seems plausible.
David Daney
>
> Ben.
>
> drivers/net/phy/bcm87xx.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/phy/bcm87xx.c b/drivers/net/phy/bcm87xx.c
> index 7997895..f5b44ff 100644
> --- a/drivers/net/phy/bcm87xx.c
> +++ b/drivers/net/phy/bcm87xx.c
> @@ -230,4 +230,11 @@ static void __exit bcm87xx_exit(void)
> }
> module_exit(bcm87xx_exit);
>
> +static struct mdio_device_id __maybe_unused bcm87xx_tbl[] = {
> + { PHY_ID_BCM8706, 0xffffffff },
> + { PHY_ID_BCM8727, 0xffffffff },
> + { }
> +};
> +
> MODULE_LICENSE("GPL");
> +MODULE_DEVICE_TABLE(mdio, bcm87xx_tbl);
>
^ permalink raw reply
* [PATCH] xen-netback: count number required slots for an skb more carefully
From: David Vrabel @ 2013-09-03 17:29 UTC (permalink / raw)
To: xen-devel
Cc: David Vrabel, Konrad Rzeszutek Wilk, Boris Ostrovsky,
Ian Campbell, netdev
From: David Vrabel <david.vrabel@citrix.com>
When a VM is providing an iSCSI target and the LUN is used by the
backend domain, the generated skbs for direct I/O writes to the disk
have large, multi-page skb->data but no frags.
With some lengths and starting offsets, xen_netbk_count_skb_slots()
would be one short because the simple calculation of
DIV_ROUND_UP(skb_headlen(), PAGE_SIZE) was not accounting for the
decisions made by start_new_rx_buffer() which does not guarantee
responses are fully packed.
For example, a skb with length < 2 pages but which spans 3 pages would
be counted as requiring 2 slots but would actually use 3 slots.
skb->data:
| 1111|222222222222|3333 |
Fully packed, this would need 2 slots:
|111122222222|22223333 |
But because the 2nd page wholy fits into a slot it is not split across
slots and goes into a slot of its own:
|1111 |222222222222|3333 |
Miscounting the number of slots means netback may push more responses
than the number of available requests. This will cause the frontend
to get very confused and report "Too many frags/slots". The frontend
never recovers and will eventually BUG.
Fix this by counting the number of required slots more carefully. In
xen_netbk_count_skb_slots(), more closely follow the algorithm used by
xen_netbk_gop_skb() by introducing xen_netbk_count_frag_slots() which
is the dry-run equivalent of netbk_gop_frag_copy().
Signed-off-by: David Vrabel <david.vrabel@citrix.com>
---
[Resend to actually Cc netdev, sorry.]
---
drivers/net/xen-netback/netback.c | 94 +++++++++++++++++++++++++------------
1 files changed, 64 insertions(+), 30 deletions(-)
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 64828de..f149ce5 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -361,6 +361,49 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
return false;
}
+struct xen_netbk_count_slot_state {
+ unsigned long copy_off;
+ bool head;
+};
+
+unsigned int xen_netbk_count_frag_slots(struct xenvif *vif,
+ unsigned long offset, unsigned long size,
+ struct xen_netbk_count_slot_state *state)
+{
+ unsigned count = 0;
+
+ offset &= ~PAGE_MASK;
+
+ while (size > 0) {
+ unsigned long bytes;
+
+ bytes = PAGE_SIZE - offset;
+
+ if (bytes > size)
+ bytes = size;
+
+ if (start_new_rx_buffer(state->copy_off, bytes, state->head)) {
+ count++;
+ state->copy_off = 0;
+ }
+
+ if (state->copy_off + bytes > MAX_BUFFER_OFFSET)
+ bytes = MAX_BUFFER_OFFSET - state->copy_off;
+
+ state->copy_off += bytes;
+
+ offset += bytes;
+ size -= bytes;
+
+ if (offset == PAGE_SIZE)
+ offset = 0;
+
+ state->head = false;
+ }
+
+ return count;
+}
+
/*
* Figure out how many ring slots we're going to need to send @skb to
* the guest. This function is essentially a dry run of
@@ -368,48 +411,39 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
*/
unsigned int xen_netbk_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
{
+ struct xen_netbk_count_slot_state state;
unsigned int count;
- int i, copy_off;
+ unsigned char *data;
+ unsigned i;
- count = DIV_ROUND_UP(skb_headlen(skb), PAGE_SIZE);
+ state.head = true;
+ state.copy_off = 0;
- copy_off = skb_headlen(skb) % PAGE_SIZE;
+ /* Slot for the first (partial) page of data. */
+ count = 1;
+ /* Need a slot for the GSO prefix for GSO extra data? */
if (skb_shinfo(skb)->gso_size)
count++;
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
- unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
- unsigned long bytes;
-
- offset &= ~PAGE_MASK;
-
- while (size > 0) {
- BUG_ON(offset >= PAGE_SIZE);
- BUG_ON(copy_off > MAX_BUFFER_OFFSET);
-
- bytes = PAGE_SIZE - offset;
-
- if (bytes > size)
- bytes = size;
+ data = skb->data;
+ while (data < skb_tail_pointer(skb)) {
+ unsigned long offset = offset_in_page(data);
+ unsigned long size = PAGE_SIZE - offset;
- if (start_new_rx_buffer(copy_off, bytes, 0)) {
- count++;
- copy_off = 0;
- }
+ if (data + size > skb_tail_pointer(skb))
+ size = skb_tail_pointer(skb) - data;
- if (copy_off + bytes > MAX_BUFFER_OFFSET)
- bytes = MAX_BUFFER_OFFSET - copy_off;
+ count += xen_netbk_count_frag_slots(vif, offset, size, &state);
- copy_off += bytes;
+ data += size;
+ }
- offset += bytes;
- size -= bytes;
+ for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+ unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
+ unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
- if (offset == PAGE_SIZE)
- offset = 0;
- }
+ count += xen_netbk_count_frag_slots(vif, offset, size, &state);
}
return count;
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH net] net: ipv6: tcp: fix potential use after free in tcp_v6_do_rcv
From: Daniel Borkmann @ 2013-09-03 17:29 UTC (permalink / raw)
To: davem; +Cc: netdev, Eric Dumazet
In tcp_v6_do_rcv() code, when processing pkt options, we soley work
on our skb clone opt_skb that we've created earlier before entering
tcp_rcv_established() on our way. However, only in condition ...
if (np->rxopt.bits.rxtclass)
np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
... we work on skb itself. As we extract every other information out
of opt_skb in ipv6_pktoptions path, this seems wrong, since skb can
already be released by tcp_rcv_established() earlier on. When we try
to access it in ipv6_hdr(), we will dereference freed skb.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
net/ipv6/tcp_ipv6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 6e1649d..eeb4cb0 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1427,7 +1427,7 @@ ipv6_pktoptions:
if (np->rxopt.bits.rxhlim || np->rxopt.bits.rxohlim)
np->mcast_hops = ipv6_hdr(opt_skb)->hop_limit;
if (np->rxopt.bits.rxtclass)
- np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(skb));
+ np->rcv_tclass = ipv6_get_dsfield(ipv6_hdr(opt_skb));
if (ipv6_opt_accepted(sk, opt_skb)) {
skb_set_owner_r(opt_skb, sk);
opt_skb = xchg(&np->pktoptions, opt_skb);
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Hannes Frederic Sowa @ 2013-09-03 17:27 UTC (permalink / raw)
To: Stephen Warren; +Cc: Thomas Graf, davem, netdev, Eric Dumazet, Fabio Estevam
In-Reply-To: <52261A12.3060203@wwwdotorg.org>
On Tue, Sep 03, 2013 at 11:19:14AM -0600, Stephen Warren wrote:
> On 09/03/2013 05:37 AM, Thomas Graf wrote:
> > Allocating skbs when sending out neighbour discovery messages
> > currently uses sock_alloc_send_skb() based on a per net namespace
> > socket and thus share a socket wmem buffer space.
> >
> > If a netdevice is temporarily unable to transmit due to carrier
> > loss or for other reasons, the queued up ndisc messages will cosnume
> > all of the wmem space and will thus prevent from any more skbs to
> > be allocated even for netdevices that are able to transmit packets.
> >
> > The number of neighbour discovery messages sent is very limited,
> > use of alloc_skb() bypasses the socket wmem buffer size enforcement
> > while the manual call to skb_set_owner_w() maintains the socket
> > reference needed for the IPv6 output path.
> >
> > This patch has orginally been posted by Eric Dumazet in a modified
> > form.
>
> Tested-by: Stephen Warren <swarren@nvidia.com>
>
> Although I do note something slightly odd:
>
> next-20130830 had an issue, and reverting V1 of this patch solved it.
>
> However, in next-20130903, if I revert the revert of V1 of this patch, I
> don't see any issue; it appears that the problem was some interaction
> between V1 of this patch and something else in next-20130830.
>
> Either way, this patch doesn't seem to introduce any issue when applied
> on top of either next-20130830 with V1 reverted, or on top of
> next-20130903, so it's fine.
Could either of you run the v1 version of the patch with CONFIG_PROVE_LOCKING
enabled? I also do think there is some side-effect we don't understand yet.
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH v2] ipv6: Don't depend on per socket memory for neighbour discovery messages
From: Stephen Warren @ 2013-09-03 17:19 UTC (permalink / raw)
To: Thomas Graf
Cc: davem, netdev, Eric Dumazet, Hannes Frederic Sowa, Fabio Estevam
In-Reply-To: <e4600dac657e1160da7a5e7758dcb973b616a10e.1378207925.git.tgraf@suug.ch>
On 09/03/2013 05:37 AM, Thomas Graf wrote:
> Allocating skbs when sending out neighbour discovery messages
> currently uses sock_alloc_send_skb() based on a per net namespace
> socket and thus share a socket wmem buffer space.
>
> If a netdevice is temporarily unable to transmit due to carrier
> loss or for other reasons, the queued up ndisc messages will cosnume
> all of the wmem space and will thus prevent from any more skbs to
> be allocated even for netdevices that are able to transmit packets.
>
> The number of neighbour discovery messages sent is very limited,
> use of alloc_skb() bypasses the socket wmem buffer size enforcement
> while the manual call to skb_set_owner_w() maintains the socket
> reference needed for the IPv6 output path.
>
> This patch has orginally been posted by Eric Dumazet in a modified
> form.
Tested-by: Stephen Warren <swarren@nvidia.com>
Although I do note something slightly odd:
next-20130830 had an issue, and reverting V1 of this patch solved it.
However, in next-20130903, if I revert the revert of V1 of this patch, I
don't see any issue; it appears that the problem was some interaction
between V1 of this patch and something else in next-20130830.
Either way, this patch doesn't seem to introduce any issue when applied
on top of either next-20130830 with V1 reverted, or on top of
next-20130903, so it's fine.
^ permalink raw reply
* Re: [PATCH] net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
From: Neil Horman @ 2013-09-03 16:46 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: linux-sctp, davem, vyasevich,
Glavinic-Pecotic, Matija (EXT-Other - DE/Ulm), netdev
In-Reply-To: <52249981.8040006@nsn.com>
On Mon, Sep 02, 2013 at 03:58:25PM +0200, Alexander Sverdlin wrote:
> From: Alexander Sverdlin <alexander.sverdlin@nsn.com>
>
> net: sctp: Fix data chunk fragmentation for MTU values which are not multiple of 4
>
> Initially the problem was observed with ipsec, but later it became clear that
> SCTP data chunk fragmentation algorithm has problems with MTU values which are
> not multiple of 4. Test program was used which just transmits 2000 bytes long
> packets to other host. tcpdump was used to observe re-fragmentation in IP layer
> after SCTP already fragmented data chunks.
>
> With MTU 1500:
> 12:54:34.082904 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 1500)
> 10.151.38.153.39303 > 10.151.24.91.54321: sctp (1) [DATA] (B) [TSN: 2366088589] [SID: 0] [SSEQ 1] [PPID 0x0]
> 12:54:34.082933 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 596)
> 10.151.38.153.39303 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 2366088590] [SID: 0] [SSEQ 1] [PPID 0x0]
> 12:54:34.090576 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.39303: sctp (1) [SACK] [cum ack 2366088590] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> With MTU 1499:
> 13:02:49.955220 IP (tos 0x2,ECT(0), ttl 64, id 48215, offset 0, flags [+], proto SCTP (132), length 1492)
> 10.151.38.153.39084 > 10.151.24.91.54321: sctp[|sctp]
> 13:02:49.955249 IP (tos 0x2,ECT(0), ttl 64, id 48215, offset 1472, flags [none], proto SCTP (132), length 28)
> 10.151.38.153 > 10.151.24.91: ip-proto-132
> 13:02:49.955262 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 600)
> 10.151.38.153.39084 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 404355346] [SID: 0] [SSEQ 1] [PPID 0x0]
> 13:02:49.956770 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.39084: sctp (1) [SACK] [cum ack 404355346] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> Here problem in data portion limit calculation leads to re-fragmentation in IP,
> which is sub-optimal. The problem is max_data initial value, which doesn't take
> into account the fact, that data chunk must be padded to 4-bytes boundary.
> It's enough to correct max_data, because all later adjustments are correctly
> aligned to 4-bytes boundary.
>
> After the fix is applied, everything is fragmented correctly for uneven MTUs:
> 15:16:27.083881 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 1496)
> 10.151.38.153.53417 > 10.151.24.91.54321: sctp (1) [DATA] (B) [TSN: 3077098183] [SID: 0] [SSEQ 1] [PPID 0x0]
> 15:16:27.083907 IP (tos 0x2,ECT(0), ttl 64, id 0, offset 0, flags [DF], proto SCTP (132), length 600)
> 10.151.38.153.53417 > 10.151.24.91.54321: sctp (1) [DATA] (E) [TSN: 3077098184] [SID: 0] [SSEQ 1] [PPID 0x0]
> 15:16:27.085640 IP (tos 0x2,ECT(0), ttl 63, id 0, offset 0, flags [DF], proto SCTP (132), length 48)
> 10.151.24.91.54321 > 10.151.38.153.53417: sctp (1) [SACK] [cum ack 3077098184] [a_rwnd 79920] [#gap acks 0] [#dup tsns 0]
>
> The bug was there for years already, but
> - is a performance issue, the packets are still transmitted
> - doesn't show up with default MTU 1500, but possibly with ipsec (MTU 1438)
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nsn.com>
> ---
>
> --- linux-3.10.orig/net/sctp/chunk.c
> +++ linux-3.10/net/sctp/chunk.c
> @@ -200,9 +200,9 @@ struct sctp_datamsg *sctp_datamsg_from_u
> /* This is the biggest possible DATA chunk that can fit into
> * the packet
> */
> - max_data = asoc->pathmtu -
> + max_data = (asoc->pathmtu -
> sctp_sk(asoc->base.sk)->pf->af->net_header_len -
> - sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
> + sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk)) & ~3;
>
> max = asoc->frag_point;
> /* If the the peer requested that we authenticate DATA chunks
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* [PATCH net-next] vxlan: Optimize vxlan rcv
From: Pravin B Shelar @ 2013-09-03 16:44 UTC (permalink / raw)
To: netdev; +Cc: stephen, Pravin B Shelar
vxlan-udp-recv function lookup vxlan_sock struct on every packet
recv by using udp-port number. we can use sk->sk_user_data to
store vxlan_sock and avoid lookup.
I have open coded rcu-api to store and read vxlan_sock from
sk_user_data to avoid sparse warning as sk_user_data is not
__rcu pointer.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
drivers/net/vxlan.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 6b560f3..7de131f 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -916,6 +916,8 @@ void vxlan_sock_release(struct vxlan_sock *vs)
spin_lock(&vn->sock_lock);
hlist_del_rcu(&vs->hlist);
+ smp_wmb();
+ vs->sock->sk->sk_user_data = NULL;
spin_unlock(&vn->sock_lock);
queue_work(vxlan_wq, &vs->del_work);
@@ -1009,7 +1011,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
port = inet_sk(sk)->inet_sport;
- vs = vxlan_find_sock(sock_net(sk), port);
+ smp_read_barrier_depends();
+ vs = (struct vxlan_sock *)sk->sk_user_data;
if (!vs)
goto drop;
@@ -2236,6 +2239,8 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
atomic_set(&vs->refcnt, 1);
vs->rcv = rcv;
vs->data = data;
+ smp_wmb();
+ vs->sock->sk->sk_user_data = vs;
spin_lock(&vn->sock_lock);
hlist_add_head_rcu(&vs->hlist, vs_head(net, port));
--
1.7.1
^ 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