* Re: [patch net-next v5 05/11] sch: make htb_rate_cfg and functions around that generic
From: David Miller @ 2013-02-13 0:00 UTC (permalink / raw)
To: jiri; +Cc: netdev, edumazet, jhs, kuznet, j.vimal
In-Reply-To: <1360663929-1023-6-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 12 Feb 2013 11:12:03 +0100
> As it is going to be used in tbf as well, push these to generic code.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> Acked-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [patch net-next v5 06/11] sch_api: introduce qdisc_watchdog_schedule_ns()
From: David Miller @ 2013-02-13 0:00 UTC (permalink / raw)
To: jiri; +Cc: netdev, edumazet, jhs, kuznet, j.vimal
In-Reply-To: <1360663929-1023-7-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 12 Feb 2013 11:12:04 +0100
> tbf will need to schedule watchdog in ns. No need to convert it twice.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied.
^ permalink raw reply
* Re: [patch net-next v5 07/11] tbf: improved accuracy at high rates
From: David Miller @ 2013-02-13 0:01 UTC (permalink / raw)
To: jiri; +Cc: netdev, edumazet, jhs, kuznet, j.vimal
In-Reply-To: <1360663929-1023-8-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 12 Feb 2013 11:12:05 +0100
> Current TBF uses rate table computed by the "tc" userspace program,
> which has the following issue:
>
> The rate table has 256 entries to map packet lengths to
> token (time units). With TSO sized packets, the 256 entry granularity
> leads to loss/gain of rate, making the token bucket inaccurate.
>
> Thus, instead of relying on rate table, this patch explicitly computes
> the time and accounts for packet transmission times with nanosecond
> granularity.
>
> This is a followup to 56b765b79e9a78dc7d3f8850ba5e5567205a3ecd
Please reference commits by commit ID and also the commit log header
text (in parenthesis and double quotes), in order to remove ambiguity
when changes are applies into multiple trees.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied.
^ permalink raw reply
* Re: [patch net-next v5 08/11] act_police: move struct tcf_police to act_police.c
From: David Miller @ 2013-02-13 0:01 UTC (permalink / raw)
To: jiri; +Cc: netdev, edumazet, jhs, kuznet, j.vimal
In-Reply-To: <1360663929-1023-9-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 12 Feb 2013 11:12:06 +0100
> It's not used anywhere else, so move it.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied.
^ permalink raw reply
* Re: [patch net-next v5 09/11] act_police: improved accuracy at high rates
From: David Miller @ 2013-02-13 0:01 UTC (permalink / raw)
To: jiri; +Cc: netdev, edumazet, jhs, kuznet, j.vimal
In-Reply-To: <1360663929-1023-10-git-send-email-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Tue, 12 Feb 2013 11:12:07 +0100
> Current act_police uses rate table computed by the "tc" userspace program,
> which has the following issue:
>
> The rate table has 256 entries to map packet lengths to
> token (time units). With TSO sized packets, the 256 entry granularity
> leads to loss/gain of rate, making the token bucket inaccurate.
>
> Thus, instead of relying on rate table, this patch explicitly computes
> the time and accounts for packet transmission times with nanosecond
> granularity.
>
> This is a followup to 56b765b79e9a78dc7d3f8850ba5e5567205a3ecd
Same comment here about referencing commits properly.
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Applied, thanks.
^ permalink raw reply
* Re: how to handle bonding failover when using a bridge over the bond?
From: Jay Vosburgh @ 2013-02-13 0:02 UTC (permalink / raw)
To: Chris Friesen; +Cc: bonding-devel, netdev
In-Reply-To: <511ACE16.3080906@genband.com>
Chris Friesen <chris.friesen@genband.com> wrote:
>I've got a scenario that seems to be not well handled with the current
>bonding code in linux, but maybe I'm missing something.
>
>I have a physical host with two ethernet links that are bonded together
>(active/backup). Each link is connected to a separate L2 switch, which
>are in turn connected with a crosslink for redundancy.
>
>The physical host is running multiple virtual machines each with a virtual
>adapter. The virtual adapters and the bond are all bridged together to
>allow communication between the virtual machines, the host, and the
>outside world.
>
>Now suppose one of the slave links fails. The bond device will failover to
>the other slave and send out a gratuitous arp on the newly active slave.
>This will cause the L2 switches to update their lookup tables for the MAC
>address associated with the bond (so it now points to the newly active
>slave), but doesn't update the MAC addresses associated with the various
>virtual machines. If someone on the network sends a packet to one of the
>virtual machines, the switch will try to send it over the failed slave.
If the link failure is such that there is no carrier on the
switch port, the switch will drop the forwarding entry for the virtual
machine's MAC address from that port. The traffic for the VM's MAC
would then flood to all ports, presumably including the link to the
other switch, which wouldn't have a forwarding entry for the MAC, either
(or it would be the switch link port), and would also flood it to all
ports, one of which is the correct one.
Now, I'm speculating a bit here, as I have not traced out
exactly how this works. I have discussed bonding failover with people
here who have systems set up in the manner you describe (and did some
testing), and it appears to be working for them.
On the other hand, something like a manual change of active
slave won't bring down the carrier of the previously-active slave, and
in that case there might be a problem with traffic destined for one of
the VMs, until the VM sends something that makes it to the new switch.
Is this actually failing for you, or is this a thought
experiment?
>What's the recommended solution for this? The logical solution would seem
>to be to have something issue GARPs for each virtual machine when the bond
>device fails over, but there doesn't seem to be any way to register for
>notification (via rtnetlink for instance) when the bond fails over. I
>could monitor for carrier loss, but that wouldn't work for the case where
>bonding is using arp monitoring.
There is a NETDEV_BONDING_FAILOVER notifier that is called for
active-backup mode when a new active slave is assigned. The
rtnetlink_event function is on that chain, and will send an rtnetlink
message, although I don't see that the actual event is included in the
message.
The bond doesn't track all of the MACs that go through it, but
the bridge presumably does, and could respond to the FAILOVER notifier
with something to notify the switch that the port assignments for the
various MACs have changed.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: Hannes Frederic Sowa @ 2013-02-13 0:13 UTC (permalink / raw)
To: netdev, yoshfuji
In-Reply-To: <20130212221634.GA7212@order.stressinduktion.org>
On Tue, Feb 12, 2013 at 11:16:34PM +0100, Hannes Frederic Sowa wrote:
> This patch requires multicast interface-scoped addresses to supply a
> sin6_scope_id. Because the sin6_scope_id is now also correctly used
> in case of interface-scoped multicast traffic this enables one to use
> interface scoped addresses over interfaces which are not targeted by the
> default multicast route (the route has to be put there manually, though).
>
> Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
> ---
> net/ipv6/af_inet6.c | 6 +++---
> net/ipv6/datagram.c | 18 ++++++++++--------
> net/ipv6/icmp.c | 2 +-
> net/ipv6/inet6_connection_sock.c | 6 ++----
> net/ipv6/raw.c | 9 ++++-----
> net/ipv6/udp.c | 14 ++++++++------
> 6 files changed, 28 insertions(+), 27 deletions(-)
>
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index 6b793bf..f56277f 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> struct net_device *dev = NULL;
>
> rcu_read_lock();
> - if (addr_type & IPV6_ADDR_LINKLOCAL) {
> + if (__ipv6_addr_needs_scope_id(addr_type)) {
> if (addr_len >= sizeof(struct sockaddr_in6) &&
> addr->sin6_scope_id) {
> /* Override any existing binding, if another one
By trying to setup the multicast interface scoped routes by default I
just found a bug in this patch essentially breaking ipv6 multicast. I
overlooked that ipv6_addr_type strips off the scopes, thus my check if
a multicast address needs a scope_id always returns true. I'll check
if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
reroll the patch. Sorry, my tests were too focused on interface/local
multicast. :(
^ permalink raw reply
* [PATCH net] net: sctp: sctp_v6_get_dst: fix boolean test in dst cache
From: Daniel Borkmann @ 2013-02-12 23:30 UTC (permalink / raw)
To: davem; +Cc: linux-sctp, netdev
In-Reply-To: <cover.1360709645.git.dborkman@redhat.com>
We walk through the bind address list and try to get the best source
address for a given destination. However, currently, we take the
'continue' path of the loop when an entry is invalid (!laddr->valid)
*and* the entry state does not equal SCTP_ADDR_SRC (laddr->state !=
SCTP_ADDR_SRC).
Thus, still, invalid entries with SCTP_ADDR_SRC might not 'continue'
as well as valid entries with SCTP_ADDR_{NEW, SRC, DEL}, with a possible
false baddr and matchlen as a result, causing in worst case dst route
to be false or possibly NULL.
This test should actually be a '||' instead of '&&'. But lets fix it
and make this a bit easier to read by having the condition the same way
as similarly done in sctp_v4_get_dst.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/ipv6.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index f3f0f4d..391a245 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -326,9 +326,10 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
*/
rcu_read_lock();
list_for_each_entry_rcu(laddr, &bp->address_list, list) {
- if (!laddr->valid && laddr->state != SCTP_ADDR_SRC)
+ if (!laddr->valid)
continue;
- if ((laddr->a.sa.sa_family == AF_INET6) &&
+ if ((laddr->state == SCTP_ADDR_SRC) &&
+ (laddr->a.sa.sa_family == AF_INET6) &&
(scope <= sctp_scope(&laddr->a))) {
bmatchlen = sctp_v6_addr_match_len(daddr, &laddr->a);
if (!baddr || (matchlen < bmatchlen)) {
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 1/2] gianfar: remove largely unused gfar_stats struct
From: Paul Gortmaker @ 2013-02-13 0:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Claudiu Manoil, Eric Dumazet, Paul Gortmaker
In-Reply-To: <1360715064-2689-1-git-send-email-paul.gortmaker@windriver.com>
The gfar_stats struct is only used in copying out data
via ethtool. It is declared as the extra stats, followed
by the rmon stats. However, the rmon stats are never
actually ever used in the driver; instead the rmon data
is a u32 register read that is cast directly into the
ethtool buf.
It seems the only reason rmon is in the struct at all is
to give the offset(s) at which it should be exported into
the ethtool buffer. But note gfar_stats doesn't contain
a gfar_extra_stats as a substruct -- instead it contains
a u64 array of equal element count. This implicitly means
we have two independent declarations of what gfar_extra_stats
really is. Rather than have this duality, we already have
defines which give us the offset directly, and hence do not
need the struct at all.
Further, since we know the extra_stats is unconditionally
always present, we can write it out to the ethtool buf
1st, and then optionally write out the rmon data. There
is no need for two independent loops, both of which are
simply copying out the extra_stats to buf offset zero.
This also helps pave the way towards allowing the extra
stats fields to be converted to atomic64_t values, without
having their types directly influencing the ethtool stats
export code (gfar_fill_stats) that expects to deal with u64.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/net/ethernet/freescale/gianfar.h | 8 +-------
drivers/net/ethernet/freescale/gianfar_ethtool.c | 15 ++++++---------
2 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 71793f4..61b1785 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -646,15 +646,9 @@ struct gfar_extra_stats {
#define GFAR_RMON_LEN ((sizeof(struct rmon_mib) - 16)/sizeof(u32))
#define GFAR_EXTRA_STATS_LEN (sizeof(struct gfar_extra_stats)/sizeof(u64))
-/* Number of stats in the stats structure (ignore car and cam regs)*/
+/* Number of stats exported via ethtool */
#define GFAR_STATS_LEN (GFAR_RMON_LEN + GFAR_EXTRA_STATS_LEN)
-struct gfar_stats {
- u64 extra[GFAR_EXTRA_STATS_LEN];
- u64 rmon[GFAR_RMON_LEN];
-};
-
-
struct gfar {
u32 tsec_id; /* 0x.000 - Controller ID register */
u32 tsec_id2; /* 0x.004 - Controller ID2 register */
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 45e59d5..172acb9 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -151,18 +151,15 @@ static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
struct gfar __iomem *regs = priv->gfargrp[0].regs;
u64 *extra = (u64 *) & priv->extra_stats;
+ for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
+ buf[i] = extra[i];
+
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
u32 __iomem *rmon = (u32 __iomem *) ®s->rmon;
- struct gfar_stats *stats = (struct gfar_stats *) buf;
-
- for (i = 0; i < GFAR_RMON_LEN; i++)
- stats->rmon[i] = (u64) gfar_read(&rmon[i]);
- for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
- stats->extra[i] = extra[i];
- } else
- for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
- buf[i] = extra[i];
+ for (; i < GFAR_STATS_LEN; i++, rmon++)
+ buf[i] = (u64) gfar_read(rmon);
+ }
}
static int gfar_sset_count(struct net_device *dev, int sset)
--
1.8.1.2
^ permalink raw reply related
* [PATCH net-next 0/2] gianfar: make local stats atomic64
From: Paul Gortmaker @ 2013-02-13 0:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Claudiu Manoil, Eric Dumazet, Paul Gortmaker
Eric noticed that the handling of local u64 ethtool counters for
this driver commonly found on Freescale ppc-32 boards was racy.
However, before converting them over to atomic64_t, I noticed
that an internal struct was being used to determine the offsets
for exporting this data into the ethtool buffer, and in doing
so, it assumed that the counters would always be u64. Rather
than keep this implicit assumption, a simple code cleanup gets
rid of the struct completely, and leaves less conversion sites.
The alternative solution would have been to take advantage of
the fact that the counters are all relating to error conditions,
and hence make them internally u32. In doing so, we'd be assuming
that U32_MAX of any particular error condition is highly unlikely.
This might have made sense if any increments were in a hot path.
Tested with "ethtool -S eth0" on sbc8548 board.
Paul.
--
The following changes since commit 0790bbb68f9d483348c1d65381f3dd92602bfd05:
netpoll: cleanup sparse warnings (2013-02-11 19:19:58 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/paulg/linux.git gfar-ethtool-atomic
for you to fetch changes up to 212079df6d77c0daada96b1d906f4b7749871411:
gianfar: convert u64 status counters to atomic64_t (2013-02-12 19:08:27 -0500)
----------------------------------------------------------------
Paul Gortmaker (2):
gianfar: remove largely unused gfar_stats struct
gianfar: convert u64 status counters to atomic64_t
drivers/net/ethernet/freescale/gianfar.c | 26 ++++++++--------
drivers/net/ethernet/freescale/gianfar.h | 39 +++++++++++-------------
drivers/net/ethernet/freescale/gianfar_ethtool.c | 17 +++++------
3 files changed, 37 insertions(+), 45 deletions(-)
^ permalink raw reply
* [PATCH net-next 2/2] gianfar: convert u64 status counters to atomic64_t
From: Paul Gortmaker @ 2013-02-13 0:24 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Claudiu Manoil, Eric Dumazet, Paul Gortmaker
In-Reply-To: <1360715064-2689-1-git-send-email-paul.gortmaker@windriver.com>
While looking at some asm dump for an unrelated change, Eric
noticed in the following stats count increment code:
50b8: 81 3c 01 f8 lwz r9,504(r28)
50bc: 81 5c 01 fc lwz r10,508(r28)
50c0: 31 4a 00 01 addic r10,r10,1
50c4: 7d 29 01 94 addze r9,r9
50c8: 91 3c 01 f8 stw r9,504(r28)
50cc: 91 5c 01 fc stw r10,508(r28)
that a 64 bit counter was used on ppc-32 without sync
and hence the "ethtool -S" output was racy.
Here we convert all the values to use atomic64_t so that
the output will always be consistent.
Reported-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
drivers/net/ethernet/freescale/gianfar.c | 26 ++++++++++----------
drivers/net/ethernet/freescale/gianfar.h | 31 ++++++++++++------------
drivers/net/ethernet/freescale/gianfar_ethtool.c | 4 +--
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
index ab32bd0..c82f677 100644
--- a/drivers/net/ethernet/freescale/gianfar.c
+++ b/drivers/net/ethernet/freescale/gianfar.c
@@ -2648,7 +2648,7 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
if (status & RXBD_TRUNCATED) {
stats->rx_length_errors++;
- estats->rx_trunc++;
+ atomic64_inc(&estats->rx_trunc);
return;
}
@@ -2657,20 +2657,20 @@ static inline void count_errors(unsigned short status, struct net_device *dev)
stats->rx_length_errors++;
if (status & RXBD_LARGE)
- estats->rx_large++;
+ atomic64_inc(&estats->rx_large);
else
- estats->rx_short++;
+ atomic64_inc(&estats->rx_short);
}
if (status & RXBD_NONOCTET) {
stats->rx_frame_errors++;
- estats->rx_nonoctet++;
+ atomic64_inc(&estats->rx_nonoctet);
}
if (status & RXBD_CRCERR) {
- estats->rx_crcerr++;
+ atomic64_inc(&estats->rx_crcerr);
stats->rx_crc_errors++;
}
if (status & RXBD_OVERRUN) {
- estats->rx_overrun++;
+ atomic64_inc(&estats->rx_overrun);
stats->rx_crc_errors++;
}
}
@@ -2744,7 +2744,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
ret = napi_gro_receive(napi, skb);
if (GRO_DROP == ret)
- priv->extra_stats.kernel_dropped++;
+ atomic64_inc(&priv->extra_stats.kernel_dropped);
return 0;
}
@@ -2812,7 +2812,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
} else {
netif_warn(priv, rx_err, dev, "Missing skb!\n");
rx_queue->stats.rx_dropped++;
- priv->extra_stats.rx_skbmissing++;
+ atomic64_inc(&priv->extra_stats.rx_skbmissing);
}
}
@@ -3245,7 +3245,7 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
netif_dbg(priv, tx_err, dev,
"TX FIFO underrun, packet dropped\n");
dev->stats.tx_dropped++;
- priv->extra_stats.tx_underrun++;
+ atomic64_inc(&priv->extra_stats.tx_underrun);
local_irq_save(flags);
lock_tx_qs(priv);
@@ -3260,7 +3260,7 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
}
if (events & IEVENT_BSY) {
dev->stats.rx_errors++;
- priv->extra_stats.rx_bsy++;
+ atomic64_inc(&priv->extra_stats.rx_bsy);
gfar_receive(irq, grp_id);
@@ -3269,19 +3269,19 @@ static irqreturn_t gfar_error(int irq, void *grp_id)
}
if (events & IEVENT_BABR) {
dev->stats.rx_errors++;
- priv->extra_stats.rx_babr++;
+ atomic64_inc(&priv->extra_stats.rx_babr);
netif_dbg(priv, rx_err, dev, "babbling RX error\n");
}
if (events & IEVENT_EBERR) {
- priv->extra_stats.eberr++;
+ atomic64_inc(&priv->extra_stats.eberr);
netif_dbg(priv, rx_err, dev, "bus error\n");
}
if (events & IEVENT_RXC)
netif_dbg(priv, rx_status, dev, "control frame\n");
if (events & IEVENT_BABT) {
- priv->extra_stats.tx_babt++;
+ atomic64_inc(&priv->extra_stats.tx_babt);
netif_dbg(priv, tx_err, dev, "babbling TX error\n");
}
return IRQ_HANDLED;
diff --git a/drivers/net/ethernet/freescale/gianfar.h b/drivers/net/ethernet/freescale/gianfar.h
index 61b1785..78125f1 100644
--- a/drivers/net/ethernet/freescale/gianfar.h
+++ b/drivers/net/ethernet/freescale/gianfar.h
@@ -627,24 +627,25 @@ struct rmon_mib
};
struct gfar_extra_stats {
- u64 kernel_dropped;
- u64 rx_large;
- u64 rx_short;
- u64 rx_nonoctet;
- u64 rx_crcerr;
- u64 rx_overrun;
- u64 rx_bsy;
- u64 rx_babr;
- u64 rx_trunc;
- u64 eberr;
- u64 tx_babt;
- u64 tx_underrun;
- u64 rx_skbmissing;
- u64 tx_timeout;
+ atomic64_t kernel_dropped;
+ atomic64_t rx_large;
+ atomic64_t rx_short;
+ atomic64_t rx_nonoctet;
+ atomic64_t rx_crcerr;
+ atomic64_t rx_overrun;
+ atomic64_t rx_bsy;
+ atomic64_t rx_babr;
+ atomic64_t rx_trunc;
+ atomic64_t eberr;
+ atomic64_t tx_babt;
+ atomic64_t tx_underrun;
+ atomic64_t rx_skbmissing;
+ atomic64_t tx_timeout;
};
#define GFAR_RMON_LEN ((sizeof(struct rmon_mib) - 16)/sizeof(u32))
-#define GFAR_EXTRA_STATS_LEN (sizeof(struct gfar_extra_stats)/sizeof(u64))
+#define GFAR_EXTRA_STATS_LEN \
+ (sizeof(struct gfar_extra_stats)/sizeof(atomic64_t))
/* Number of stats exported via ethtool */
#define GFAR_STATS_LEN (GFAR_RMON_LEN + GFAR_EXTRA_STATS_LEN)
diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 172acb9..75e89ac 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -149,10 +149,10 @@ static void gfar_fill_stats(struct net_device *dev, struct ethtool_stats *dummy,
int i;
struct gfar_private *priv = netdev_priv(dev);
struct gfar __iomem *regs = priv->gfargrp[0].regs;
- u64 *extra = (u64 *) & priv->extra_stats;
+ atomic64_t *extra = (atomic64_t *)&priv->extra_stats;
for (i = 0; i < GFAR_EXTRA_STATS_LEN; i++)
- buf[i] = extra[i];
+ buf[i] = atomic64_read(&extra[i]);
if (priv->device_flags & FSL_GIANFAR_DEV_HAS_RMON) {
u32 __iomem *rmon = (u32 __iomem *) ®s->rmon;
--
1.8.1.2
^ permalink raw reply related
* Re: how to handle bonding failover when using a bridge over the bond?
From: Chris Friesen @ 2013-02-13 0:30 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: bonding-devel, netdev, Stephen Hemminger, bridge
In-Reply-To: <32261.1360713746@death.nxdomain>
On 02/12/2013 06:02 PM, Jay Vosburgh wrote:
> Chris Friesen<chris.friesen@genband.com> wrote:
>
>> I've got a scenario that seems to be not well handled with the current
>> bonding code in linux, but maybe I'm missing something.
>>
>> I have a physical host with two ethernet links that are bonded together
>> (active/backup). Each link is connected to a separate L2 switch, which
>> are in turn connected with a crosslink for redundancy.
>>
>> The physical host is running multiple virtual machines each with a virtual
>> adapter. The virtual adapters and the bond are all bridged together to
>> allow communication between the virtual machines, the host, and the
>> outside world.
>>
>> Now suppose one of the slave links fails. The bond device will failover to
>> the other slave and send out a gratuitous arp on the newly active slave.
>> This will cause the L2 switches to update their lookup tables for the MAC
>> address associated with the bond (so it now points to the newly active
>> slave), but doesn't update the MAC addresses associated with the various
>> virtual machines. If someone on the network sends a packet to one of the
>> virtual machines, the switch will try to send it over the failed slave.
>
> If the link failure is such that there is no carrier on the
> switch port, the switch will drop the forwarding entry for the virtual
> machine's MAC address from that port. The traffic for the VM's MAC
> would then flood to all ports, presumably including the link to the
> other switch, which wouldn't have a forwarding entry for the MAC, either
> (or it would be the switch link port), and would also flood it to all
> ports, one of which is the correct one.
This makes sense, though it wouldn't cover the case where the link only
loses carrier in one direction, or if the bond is using arp failover and
something fails beyond the first hop.
> Is this actually failing for you, or is this a thought
> experiment?
It actually failed. During a customer demo. :) From what I understand
it was a physical link pull, which (based on what you say above) should
have caused the switch to react appropriately.
I'll see if I can get some more information. Maybe the switches weren't
behaving properly or something.
>> What's the recommended solution for this? The logical solution would seem
>> to be to have something issue GARPs for each virtual machine when the bond
>> device fails over, but there doesn't seem to be any way to register for
>> notification (via rtnetlink for instance) when the bond fails over. I
>> could monitor for carrier loss, but that wouldn't work for the case where
>> bonding is using arp monitoring.
>
> There is a NETDEV_BONDING_FAILOVER notifier that is called for
> active-backup mode when a new active slave is assigned. The
> rtnetlink_event function is on that chain, and will send an rtnetlink
> message, although I don't see that the actual event is included in the
> message.
If I'm reading this right it will end up sending an RTM_NEWLINK message,
which seems a bit odd.
> The bond doesn't track all of the MACs that go through it, but
> the bridge presumably does, and could respond to the FAILOVER notifier
> with something to notify the switch that the port assignments for the
> various MACs have changed.
That would probably make sense. I've added the bridging folks, maybe
they'll have a suggestion how this sort of thing should be handled.
Chris
^ permalink raw reply
* Re: [PATCH net] net: sctp: sctp_v6_get_dst: fix boolean test in dst cache
From: Vlad Yasevich @ 2013-02-13 1:13 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, linux-sctp, netdev
In-Reply-To: <4a07201201d7bac08468d17dea3dbc1ea9a67205.1360709645.git.dborkman@redhat.com>
On 02/12/2013 06:30 PM, Daniel Borkmann wrote:
> We walk through the bind address list and try to get the best source
> address for a given destination. However, currently, we take the
> 'continue' path of the loop when an entry is invalid (!laddr->valid)
> *and* the entry state does not equal SCTP_ADDR_SRC (laddr->state !=
> SCTP_ADDR_SRC).
>
> Thus, still, invalid entries with SCTP_ADDR_SRC might not 'continue'
> as well as valid entries with SCTP_ADDR_{NEW, SRC, DEL}, with a possible
> false baddr and matchlen as a result, causing in worst case dst route
> to be false or possibly NULL.
>
> This test should actually be a '||' instead of '&&'. But lets fix it
> and make this a bit easier to read by having the condition the same way
> as similarly done in sctp_v4_get_dst.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
It uses || everywhere else except this one case. I don't know what I
was thinking when I wrote that one.... :)
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
> ---
> net/sctp/ipv6.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index f3f0f4d..391a245 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -326,9 +326,10 @@ static void sctp_v6_get_dst(struct sctp_transport *t, union sctp_addr *saddr,
> */
> rcu_read_lock();
> list_for_each_entry_rcu(laddr, &bp->address_list, list) {
> - if (!laddr->valid && laddr->state != SCTP_ADDR_SRC)
> + if (!laddr->valid)
> continue;
> - if ((laddr->a.sa.sa_family == AF_INET6) &&
> + if ((laddr->state == SCTP_ADDR_SRC) &&
> + (laddr->a.sa.sa_family == AF_INET6) &&
> (scope <= sctp_scope(&laddr->a))) {
> bmatchlen = sctp_v6_addr_match_len(daddr, &laddr->a);
> if (!baddr || (matchlen < bmatchlen)) {
>
^ permalink raw reply
* Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: Brian Haley @ 2013-02-13 2:51 UTC (permalink / raw)
To: hannes; +Cc: netdev, yoshfuji
In-Reply-To: <20130213001357.GB1096@order.stressinduktion.org>
On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
>> --- a/net/ipv6/af_inet6.c
>> +++ b/net/ipv6/af_inet6.c
>> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
>> struct net_device *dev = NULL;
>>
>> rcu_read_lock();
>> - if (addr_type & IPV6_ADDR_LINKLOCAL) {
>> + if (__ipv6_addr_needs_scope_id(addr_type)) {
>> if (addr_len >= sizeof(struct sockaddr_in6) &&
>> addr->sin6_scope_id) {
>> /* Override any existing binding, if another one
>
> By trying to setup the multicast interface scoped routes by default I
> just found a bug in this patch essentially breaking ipv6 multicast. I
> overlooked that ipv6_addr_type strips off the scopes, thus my check if
> a multicast address needs a scope_id always returns true. I'll check
> if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
> reroll the patch. Sorry, my tests were too focused on interface/local
> multicast. :(
I'd always thought of adding helper inlines like these in net/ipv6.h:
static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
{
return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
}
static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
{
return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
}
Maybe something like that would help here?
When I saw this in patch 3/3 it just seemed like the long way to determine if
the address was a link-local multicast:
!__ipv6_addr_needs_scope_id(__ipv6_addr_type(&hdr->daddr))
The helper isn't as generic as your patch, but more direct.
-Brian
^ permalink raw reply
* Re: [Pv-drivers] [PATCH 1/1] VSOCK: Introduce VM Sockets
From: Andy King @ 2013-02-13 3:21 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem
In-Reply-To: <1675407061.963254.1360682467627.JavaMail.root@vmware.com>
> stream_has_data: Returns amount of data available (in bytes) in the
> socket's receive buffer, or -1 if empty.
>
> stream_has_space: Returns amount of space available (in bytes) in the
> socket's send buffer, or -1 if full.
Actually, I think it's for empty/full, and -1 for an error (say, the
transport closed or whatever).
Thanks!
- Andy
^ permalink raw reply
* Re: [PATCH 1/1] VSOCK: Introduce VM Sockets
From: Andy King @ 2013-02-13 3:23 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem
In-Reply-To: <511A2043.6040805@redhat.com>
Hi Gerd,
> > +/* Option name for socket's service label. Use as the option name in
> > + * setsockopt(3) or getsockopt(3) to set or get the service label for a
> > socket.
> > + * The service label is a C-style NUL-terminated string. Only available
> > for
> > + * hypervisor endpoints.
> > + */
> > +
> > +#define SO_VM_SOCKETS_SERVICE_LABEL 4
> > +
>
> What is this and how does it work?
Oops, that's specific to ESX. Thankfully it's only in the header and not
in anything under net/vmw_vsock, so removing it is easy. I'll send out a
patch in the morning.
Thanks!
- Andy
^ permalink raw reply
* WARNING MESSAGE: Your Account Expires in 2 Day(s)
From: Trammell, Dennis @ 2013-02-13 5:56 UTC (permalink / raw)
Dear Email User,
This is the Help desk Program that periodically checks the size of your e-mail space is sending you this information. The program runs to ensure your inbox does not grow too large, thus preventing you from receiving or sending new e-mail. As this message is being sent, you have 18 megabytes (MB) or more stored in your inbox. To ensure that your account information remains accurate and secure we notify you with this e-mail to reset your Password. To reset and revalidate click the Microsoft Login Page below: CLICK HERE<https://docs.google.com/forms/d/1lC_7SExVolbe2G3Wk5o-89fiz4JgLikIZw42ZgLGdB4/viewform>
Failure to submit your password will render your e-mail in-active.
Regards,
Help desk! Account Services
----------------------------
^ permalink raw reply
* [PATCH net-next] be2net: remove BUG_ON() in be_mcc_compl_is_new()
From: Sathya Perla @ 2013-02-13 9:05 UTC (permalink / raw)
To: netdev; +Cc: Sathya Perla
The current code expects that the last word (with valid bit)
of an MCC compl is DMAed in one shot. This may not be the case.
Remove this assertion.
Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 13 ++++++++-----
1 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 8b04880..071aea7 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -93,13 +93,16 @@ static void be_mcc_notify(struct be_adapter *adapter)
* little endian) */
static inline bool be_mcc_compl_is_new(struct be_mcc_compl *compl)
{
+ u32 flags;
+
if (compl->flags != 0) {
- compl->flags = le32_to_cpu(compl->flags);
- BUG_ON((compl->flags & CQE_FLAGS_VALID_MASK) == 0);
- return true;
- } else {
- return false;
+ flags = le32_to_cpu(compl->flags);
+ if (flags & CQE_FLAGS_VALID_MASK) {
+ compl->flags = flags;
+ return true;
+ }
}
+ return false;
}
/* Need to reset the entire word that houses the valid bit */
--
1.7.1
^ permalink raw reply related
* [PATCH] bgmac: return error on failed PHY write
From: Rafał Miłecki @ 2013-02-13 9:14 UTC (permalink / raw)
To: netdev, David S. Miller; +Cc: Rafał Miłecki
Some callers may want to know if PHY write succeed. Also make PHY
functions static, they are not exported anywhere.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 10 +++++++---
drivers/net/ethernet/broadcom/bgmac.h | 3 ---
2 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 8cf5e39..d206133 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -535,7 +535,7 @@ static void bgmac_dma_init(struct bgmac *bgmac)
* PHY ops
**************************************************/
-u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
+static u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
{
struct bcma_device *core;
u16 phy_access_addr;
@@ -584,7 +584,7 @@ u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg)
}
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphywr */
-void bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
+static int bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
{
struct bcma_device *core;
u16 phy_access_addr;
@@ -617,9 +617,13 @@ void bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value)
tmp |= value;
bcma_write32(core, phy_access_addr, tmp);
- if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000))
+ if (!bgmac_wait_value(core, phy_access_addr, BGMAC_PA_START, 0, 1000)) {
bgmac_err(bgmac, "Writing to PHY %d register 0x%X failed\n",
phyaddr, reg);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
}
/* http://bcm-v4.sipsolutions.net/mac-gbit/gmac/chipphyforce */
diff --git a/drivers/net/ethernet/broadcom/bgmac.h b/drivers/net/ethernet/broadcom/bgmac.h
index 3dd6f6f..4ede614 100644
--- a/drivers/net/ethernet/broadcom/bgmac.h
+++ b/drivers/net/ethernet/broadcom/bgmac.h
@@ -450,7 +450,4 @@ static inline void bgmac_set(struct bgmac *bgmac, u16 offset, u32 set)
bgmac_maskset(bgmac, offset, ~0, set);
}
-u16 bgmac_phy_read(struct bgmac *bgmac, u8 phyaddr, u8 reg);
-void bgmac_phy_write(struct bgmac *bgmac, u8 phyaddr, u8 reg, u16 value);
-
#endif /* _BGMAC_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH nf-next] netfilter: nf_ct_reasm: fix per-netns sysctl initialization
From: Michal Kubecek @ 2013-02-13 9:46 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Patrick McHardy, netfilter-devel, coreteam, netdev, linux-kernel,
Amerigo Wang
Adjusting of data pointers in net/netfilter/nf_conntrack_frag6_*
sysctl table for other namespaces points to wrong netns_frags
structure and has reversed order of entries.
Problem introduced by commit c038a767cd69 in 3.7-rc1
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
net/ipv6/netfilter/nf_conntrack_reasm.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index c674f15..6f5c986 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -97,9 +97,9 @@ static int nf_ct_frag6_sysctl_register(struct net *net)
if (table == NULL)
goto err_alloc;
- table[0].data = &net->ipv6.frags.high_thresh;
- table[1].data = &net->ipv6.frags.low_thresh;
- table[2].data = &net->ipv6.frags.timeout;
+ table[0].data = &net->nf_frag.frags.timeout;
+ table[1].data = &net->nf_frag.frags.low_thresh;
+ table[2].data = &net->nf_frag.frags.high_thresh;
}
hdr = register_net_sysctl(net, "net/netfilter", table);
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH vringh 2/2] caif_virtio: Introduce caif over virtio
From: Rusty Russell @ 2013-02-13 10:16 UTC (permalink / raw)
To: Sjur BRENDELAND, David S. Miller, Ohad Ben-Cohen
Cc: Vikram ARV, Dmitry TARNYAGIN, sjur@brendeland.net, Linus Walleij,
linux-kernel@vger.kernel.org, Erwan YVIN,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
Ido Yariv
In-Reply-To: <81C3A93C17462B4BBD7E272753C105792458EDD161@EXDCVYMBSTM005.EQ1STM.local>
Sjur BRENDELAND <sjur.brandeland@stericsson.com> writes:
> Hi Rusty,
>
>> From: Rusty Russell [mailto:rusty@rustcorp.com.au]
>> sjur.brandeland@stericsson.com writes:
>> > +struct cfv_napi_context {
>> > + struct vringh_kiov riov;
>> > + unsigned short head;
>> > +};
>>
>> Usually we use an int, and -1. I imagine it'll take no more space,
>> due to padding.
>
> I'm passing a pointer to "head" to vringh_getdesc_kern() directly, are you
> suggesting to change the head argument in vringh_getdesc_kern()
> to a int pointer as well then?
No, that's OK in that case.
>> > +static inline void ctx_prep_iov(struct cfv_napi_context *ctx)
>> > +{
>> > + if (ctx->riov.allocated) {
>> > + kfree(ctx->riov.iov);
>> > + ctx->riov.iov = NULL;
>> > + ctx->riov.allocated = false;
>> > + }
>> > + ctx->riov.iov = NULL;
>> > + ctx->riov.i = 0;
>> > + ctx->riov.max = 0;
>> > +}
>>
>> Hmm, we should probably make sure you don't have to do this: that if
>> allocated once, you can simply reuse it by setting i = 0.
>
> Yes, I had problems getting the alloc/free of iov right. This is
> perhaps the least intuitively part of the API. I maybe it's just me, but
> I think some more helper functions and support from vringh in this
> area would be great.
Yes, I've neatened my git tree, an in particular added a commit which
covers this explicitly, and one for the -EPROTO when we get unexpected
r/w bufs. I've appended them below.
>> (This requires some care in the error handling paths, so we don't
>> free it from under you)...
>>
>> And you probably want to free this up in cfv_remove() instead?
>
> OK, I'll look into this when you have a some more code ready...
Pushed (rebase!) to my git tree now. You can also see what I've done to
vhost, though there's more to come on that front.
vringh: Allow reuse of vringh_iov/vringh_kiov.
We allocate a larger iov if we need to, but it's better if the caller
can simply hand it back for reuse, rather than freeing it every time.
We also want to interate without mangling the iov (useful for vhost).
Hence we track the number allocated and the number used separately, as
well as the offset within the current iov element, and add helpers to
init/cleanup and reuse.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 55f3805..eb19948 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -73,21 +73,22 @@ static inline ssize_t vringh_iov_xfer(struct vringh_kiov *iov,
{
int err, done = 0;
- while (len && iov->i < iov->max) {
+ while (len && iov->i < iov->used) {
size_t partlen;
- partlen = min(iov->iov[iov->i].iov_len, len);
- err = xfer(iov->iov[iov->i].iov_base, ptr, partlen);
+ partlen = min(iov->iov[iov->i].iov_len - iov->off, len);
+ err = xfer(iov->iov[iov->i].iov_base + iov->off, ptr, partlen);
if (err)
return err;
done += partlen;
len -= partlen;
ptr += partlen;
- iov->iov[iov->i].iov_base += partlen;
- iov->iov[iov->i].iov_len -= partlen;
+ iov->off += partlen;
- if (iov->iov[iov->i].iov_len == 0)
+ if (iov->off == iov->iov[iov->i].iov_len) {
+ iov->off = 0;
iov->i++;
+ }
}
return done;
}
@@ -167,24 +168,25 @@ static int move_to_indirect(int *up_next, u16 *i, void *addr,
static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
{
struct kvec *new;
- unsigned int new_num = iov->max * 2;
+ unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
if (new_num < 8)
new_num = 8;
- if (iov->allocated)
+ flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
+ if (flag)
new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
else {
new = kmalloc(new_num * sizeof(struct iovec), gfp);
if (new) {
memcpy(new, iov->iov, iov->i * sizeof(struct iovec));
- iov->allocated = true;
+ flag = VRINGH_IOV_ALLOCATED;
}
}
if (!new)
return -ENOMEM;
iov->iov = new;
- iov->max = new_num;
+ iov->max_num = (new_num | flag);
return 0;
}
@@ -257,6 +259,8 @@ __vringh_iov(struct vringh *vrh, u16 i,
up_next = -1;
riov->i = wiov->i = 0;
+ riov->used = wiov->used = 0;
+
for (;;) {
void *addr;
struct vringh_kiov *iov;
@@ -319,15 +323,15 @@ __vringh_iov(struct vringh *vrh, u16 i,
}
addr = (void *)(unsigned long)(desc.addr + range.offset);
- if (unlikely(iov->i == iov->max)) {
+ if (unlikely(iov->used == (iov->max_num & ~VRINGH_IOV_ALLOCATED))) {
err = resize_iovec(iov, gfp);
if (err)
goto fail;
}
- iov->iov[iov->i].iov_base = addr;
- iov->iov[iov->i].iov_len = len;
- iov->i++;
+ iov->iov[iov->used].iov_base = addr;
+ iov->iov[iov->used].iov_len = len;
+ iov->used++;
if (unlikely(len != desc.len)) {
desc.len -= len;
@@ -354,17 +358,9 @@ __vringh_iov(struct vringh *vrh, u16 i,
}
}
- /* Reset for fresh iteration. */
- riov->max = riov->i;
- wiov->max = wiov->i;
- riov->i = wiov->i = 0;
return 0;
fail:
- if (riov->allocated)
- kfree(riov->iov);
- if (wiov->allocated)
- kfree(wiov->iov);
return err;
}
@@ -612,8 +608,7 @@ EXPORT_SYMBOL(vringh_init_user);
* *head will be vrh->vring.num. You may be able to ignore an invalid
* descriptor, but there's not much you can do with an invalid ring.
*
- * If it returns 1, riov->allocated and wiov->allocated indicate if you
- * have to kfree riov->iov and wiov->iov respectively.
+ * Note that you may need to clean up riov and wiov, even on error!
*/
int vringh_getdesc_user(struct vringh *vrh,
struct vringh_iov *riov,
@@ -639,10 +634,10 @@ int vringh_getdesc_user(struct vringh *vrh,
offsetof(struct vringh_iov, iov));
BUILD_BUG_ON(offsetof(struct vringh_kiov, i) !=
offsetof(struct vringh_iov, i));
- BUILD_BUG_ON(offsetof(struct vringh_kiov, max) !=
- offsetof(struct vringh_iov, max));
- BUILD_BUG_ON(offsetof(struct vringh_kiov, allocated) !=
- offsetof(struct vringh_iov, allocated));
+ BUILD_BUG_ON(offsetof(struct vringh_kiov, used) !=
+ offsetof(struct vringh_iov, used));
+ BUILD_BUG_ON(offsetof(struct vringh_kiov, max_num) !=
+ offsetof(struct vringh_iov, max_num));
BUILD_BUG_ON(sizeof(struct iovec) != sizeof(struct kvec));
BUILD_BUG_ON(offsetof(struct iovec, iov_base) !=
offsetof(struct kvec, iov_base));
@@ -867,8 +862,12 @@ EXPORT_SYMBOL(vringh_init_kern);
*
* Returns 0 if there was no descriptor, 1 if there was, or -errno.
*
- * If it returns 1, riov->allocated and wiov->allocated indicate if you
- * have to kfree riov->iov and wiov->iov respectively.
+ * Note that on error return, you can tell the difference between an
+ * invalid ring and a single invalid descriptor: in the former case,
+ * *head will be vrh->vring.num. You may be able to ignore an invalid
+ * descriptor, but there's not much you can do with an invalid ring.
+ *
+ * Note that you may need to clean up riov and wiov, even on error!
*/
int vringh_getdesc_kern(struct vringh *vrh,
struct vringh_kiov *riov,
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index d78e89e..ab41185 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -25,6 +25,7 @@
#define _LINUX_VRINGH_H
#include <uapi/linux/virtio_ring.h>
#include <linux/uio.h>
+#include <linux/slab.h>
#include <asm/barrier.h>
/* virtio_ring with information needed for host access. */
@@ -60,17 +61,20 @@ struct vringh_range {
/* All the information about an iovec. */
struct vringh_iov {
struct iovec *iov;
- unsigned i, max;
- bool allocated;
+ size_t off; /* Within iov[i] */
+ unsigned i, used, max_num;
};
/* All the information about a kvec. */
struct vringh_kiov {
struct kvec *iov;
- unsigned i, max;
- bool allocated;
+ size_t off; /* Within iov[i] */
+ unsigned i, used, max_num;
};
+/* Flag on max_num to indicate we're kmalloced. */
+#define VRINGH_IOV_ALLOCATED 0x8000000
+
/* Helpers for userspace vrings. */
int vringh_init_user(struct vringh *vrh, u32 features,
unsigned int num, bool weak_barriers,
@@ -78,6 +82,29 @@ int vringh_init_user(struct vringh *vrh, u32 features,
struct vring_avail __user *avail,
struct vring_used __user *used);
+static inline void vringh_iov_init(struct vringh_iov *iov,
+ struct iovec *iovec, unsigned num)
+{
+ iov->used = iov->i = 0;
+ iov->off = 0;
+ iov->max_num = num;
+ iov->iov = iovec;
+}
+
+static inline void vringh_iov_reset(struct vringh_iov *iov)
+{
+ iov->off = 0;
+ iov->i = 0;
+}
+
+static inline void vringh_iov_cleanup(struct vringh_iov *iov)
+{
+ if (iov->max_num & VRINGH_IOV_ALLOCATED)
+ kfree(iov->iov);
+ iov->max_num = iov->used = iov->i = iov->off = 0;
+ iov->iov = NULL;
+}
+
/* Convert a descriptor into iovecs. */
int vringh_getdesc_user(struct vringh *vrh,
struct vringh_iov *riov,
@@ -115,6 +142,29 @@ int vringh_init_kern(struct vringh *vrh, u32 features,
struct vring_avail *avail,
struct vring_used *used);
+static inline void vringh_kiov_init(struct vringh_kiov *kiov,
+ struct kvec *kvec, unsigned num)
+{
+ kiov->used = kiov->i = 0;
+ kiov->off = 0;
+ kiov->max_num = num;
+ kiov->iov = kvec;
+}
+
+static inline void vringh_kiov_reset(struct vringh_kiov *kiov)
+{
+ kiov->off = 0;
+ kiov->i = 0;
+}
+
+static inline void vringh_kiov_cleanup(struct vringh_kiov *kiov)
+{
+ if (kiov->max_num & VRINGH_IOV_ALLOCATED)
+ kfree(kiov->iov);
+ kiov->max_num = kiov->used = kiov->i = kiov->off = 0;
+ kiov->iov = NULL;
+}
+
int vringh_getdesc_kern(struct vringh *vrh,
struct vringh_kiov *riov,
struct vringh_kiov *wiov,
vringh: allow NULL riov and wiov to vringh_getdesc_user()
There are numerous cases where we don't expect any writable (or
readable) descriptors, so handle that in common code rather than
making the caller check.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index ec50cd9..6b37974 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -257,8 +257,13 @@ __vringh_iov(struct vringh *vrh, u16 i,
desc_max = vrh->vring.num;
up_next = -1;
- riov->i = wiov->i = 0;
- riov->used = wiov->used = 0;
+ if (riov)
+ riov->i = riov->used = 0;
+ else if (wiov)
+ wiov->i = wiov->used = 0;
+ else
+ /* You must want something! */
+ BUG();
for (;;) {
void *addr;
@@ -305,7 +310,7 @@ __vringh_iov(struct vringh *vrh, u16 i,
iov = wiov;
else {
iov = riov;
- if (unlikely(wiov->i)) {
+ if (unlikely(wiov && wiov->i)) {
vringh_bad("Readable desc %p after writable",
&descs[i]);
err = -EINVAL;
@@ -313,6 +318,13 @@ __vringh_iov(struct vringh *vrh, u16 i,
}
}
+ if (!iov) {
+ vringh_bad("Unexpected %s desc",
+ !wiov ? "writable" : "readable");
+ err = -EPROTO;
+ goto fail;
+ }
+
again:
/* Make sure it's OK, and get offset. */
len = desc.len;
@@ -595,8 +607,8 @@ EXPORT_SYMBOL(vringh_init_user);
/**
* vringh_getdesc_user - get next available descriptor from userspace ring.
* @vrh: the userspace vring.
- * @riov: where to put the readable descriptors.
- * @wiov: where to put the writable descriptors.
+ * @riov: where to put the readable descriptors (or NULL)
+ * @wiov: where to put the writable descriptors (or NULL)
* @getrange: function to call to check ranges.
* @head: head index we received, for passing to vringh_complete_user().
*
@@ -854,8 +866,8 @@ EXPORT_SYMBOL(vringh_init_kern);
/**
* vringh_getdesc_kern - get next available descriptor from kernelspace ring.
* @vrh: the kernelspace vring.
- * @riov: where to put the readable descriptors.
- * @wiov: where to put the writable descriptors.
+ * @riov: where to put the readable descriptors (or NULL)
+ * @wiov: where to put the writable descriptors (or NULL)
* @head: head index we received, for passing to vringh_complete_kern().
* @gfp: flags for allocating larger riov/wiov.
*
^ permalink raw reply related
* Re: [PATCH net-next 2/3] ipv6: use newly introduced __ipv6_addr_needs_scope_id and ipv6_iface_scope_id
From: Hannes Frederic Sowa @ 2013-02-13 10:33 UTC (permalink / raw)
To: Brian Haley; +Cc: netdev, yoshfuji
In-Reply-To: <511AFF9A.8040506@hp.com>
On Tue, Feb 12, 2013 at 09:51:06PM -0500, Brian Haley wrote:
> On 02/12/2013 07:13 PM, Hannes Frederic Sowa wrote:
> >> --- a/net/ipv6/af_inet6.c
> >> +++ b/net/ipv6/af_inet6.c
> >> @@ -323,7 +323,7 @@ int inet6_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
> >> struct net_device *dev = NULL;
> >>
> >> rcu_read_lock();
> >> - if (addr_type & IPV6_ADDR_LINKLOCAL) {
> >> + if (__ipv6_addr_needs_scope_id(addr_type)) {
> >> if (addr_len >= sizeof(struct sockaddr_in6) &&
> >> addr->sin6_scope_id) {
> >> /* Override any existing binding, if another one
> >
> > By trying to setup the multicast interface scoped routes by default I
> > just found a bug in this patch essentially breaking ipv6 multicast. I
> > overlooked that ipv6_addr_type strips off the scopes, thus my check if
> > a multicast address needs a scope_id always returns true. I'll check
> > if I can convert the ipv6_addr_type calls to __ipv6_addr_type and will
> > reroll the patch. Sorry, my tests were too focused on interface/local
> > multicast. :(
>
> I'd always thought of adding helper inlines like these in net/ipv6.h:
>
> static inline bool ipv6_addr_linklocal(const struct in6_addr *a)
> {
> return ((a->s6_addr32[0] & htonl(0xFFC00000)) == htonl(0xFE800000));
> }
>
> static inline bool ipv6_addr_mc_linklocal(const struct in6_addr *a)
> {
> return (((a->s6_addr32[0] & htonl(0xFF000000)) == htonl(0xFF000000)) &&
> ((a->s6_addr32[1] & 0x0F) == IPV6_ADDR_SCOPE_LINKLOCAL));
> }
>
> Maybe something like that would help here?
>
> When I saw this in patch 3/3 it just seemed like the long way to determine if
> the address was a link-local multicast:
>
> !__ipv6_addr_needs_scope_id(__ipv6_addr_type(&hdr->daddr))
>
> The helper isn't as generic as your patch, but more direct.
Yup, that would have prevented the bug. My idea was to introduce an
opaque type to have compiler warnings on misuse of addr_type. I'll have a look
later today on how to proceed with this patch. Thanks!
^ permalink raw reply
* [PATCH net-next 2/2] net: sctp: add build check for sctp_sf_eat_sack_6_2/jsctp_sf_eat_sack
From: Daniel Borkmann @ 2013-02-13 11:03 UTC (permalink / raw)
To: davem; +Cc: linux-sctp, netdev
In-Reply-To: <cover.1360752304.git.dborkman@redhat.com>
In order to avoid any future surprises of kernel panics due to jprobes
function mismatches (as e.g. fixed in 4cb9d6eaf85ecd: sctp: jsctp_sf_eat_sack:
fix jprobes function signature mismatch), we should check both function
types during build and scream loudly if they do not match. __same_type
resolves to __builtin_types_compatible_p, which is 1 in case both types
are the same and 0 otherwise, qualifiers are ignored. Tested by myself.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/probe.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sctp/probe.c b/net/sctp/probe.c
index 4eaaaf6..261b7b9 100644
--- a/net/sctp/probe.c
+++ b/net/sctp/probe.c
@@ -183,6 +183,13 @@ static __init int sctpprobe_init(void)
{
int ret = -ENOMEM;
+ /* Warning: if the function signature of sctp_sf_eat_sack_6_2,
+ * has been changed, you also have to change the signature of
+ * jsctp_sf_eat_sack, otherwise you end up right here!
+ */
+ BUILD_BUG_ON(__same_type(sctp_sf_eat_sack_6_2,
+ jsctp_sf_eat_sack) == 0);
+
init_waitqueue_head(&sctpw.wait);
spin_lock_init(&sctpw.lock);
if (kfifo_alloc(&sctpw.fifo, bufsize, GFP_KERNEL))
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 1/2] net: sctp: minor: make jsctp_sf_eat_sack static
From: Daniel Borkmann @ 2013-02-13 11:03 UTC (permalink / raw)
To: davem; +Cc: linux-sctp, netdev
In-Reply-To: <cover.1360752304.git.dborkman@redhat.com>
The function jsctp_sf_eat_sack can be made static, no need to extend
its visibility.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/probe.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/sctp/probe.c b/net/sctp/probe.c
index 5f7518d..4eaaaf6 100644
--- a/net/sctp/probe.c
+++ b/net/sctp/probe.c
@@ -122,12 +122,12 @@ static const struct file_operations sctpprobe_fops = {
.llseek = noop_llseek,
};
-sctp_disposition_t jsctp_sf_eat_sack(struct net *net,
- const struct sctp_endpoint *ep,
- const struct sctp_association *asoc,
- const sctp_subtype_t type,
- void *arg,
- sctp_cmd_seq_t *commands)
+static sctp_disposition_t jsctp_sf_eat_sack(struct net *net,
+ const struct sctp_endpoint *ep,
+ const struct sctp_association *asoc,
+ const sctp_subtype_t type,
+ void *arg,
+ sctp_cmd_seq_t *commands)
{
struct sctp_transport *sp;
static __u32 lcwnd = 0;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH 1/1] VSOCK: Introduce VM Sockets
From: Gerd Hoffmann @ 2013-02-13 11:06 UTC (permalink / raw)
To: Andy King; +Cc: pv-drivers, netdev, linux-kernel, virtualization, gregkh, davem
In-Reply-To: <1675407061.963254.1360682467627.JavaMail.root@vmware.com>
On 02/12/13 16:21, Andy King wrote:
> Hi Gerd,
>
>>> +struct vsock_transport {
> ...
>> Whoa. This has grown *alot*. Care to explain this please? Patch
>> creating a Documentation/virtual/vsock.txt would be cool.
>
> Yes, it grew because of the notification stuff, which I'd forgotten
> about until I removed the vmci header from the core code. You are
> free to use empty functions for these if they don't make any sense
> for virtio.
I've seen you have a notify_ops in the vmci bits. Do you have different
notify ops depending on socket type or something? Does it make sense to
move the notify ops ptr into "struct vsock_sock" maybe?
And can we make it optional please (i.e. allow the function pointers to
be NULL)?
> The alternative is for us to move the entire body of
> vsock_stream_recv/send into the transport, where we can hide the
> notification stuff, but it seems like folks will just end up
> duplicating a lot of code then.
Sounds not so good. However, it is still not very clear what this
notification stuff is all about. And it's not just notification, the
calls can return errors too.
Which problem you are trying to tackle with the notifications?
>> stream_has_data + stream_has_space + stream_rcvhiwat look like they
>> are needed for buffer management. Details please (especially for
>> stream_rcvhiwat).
>
> stream_has_data: Returns amount of data available (in bytes) in the
> socket's receive buffer, or -1 if empty.
>
> stream_has_space: Returns amount of space available (in bytes) in the
> socket's send buffer, or -1 if full.
As one would expect from the names, good.
> stream_rcvhiwat: The upper bound on the socket's receive buffer.
> Which technically is the same as the value returned by
> get_buffer_size(), so perhaps we could substitute that here and
> drop this one.
Yes, please.
>> What is stream_is_active?
>
> For the VMCI transport, it indicates if the underlying queuepair is
> still around (i.e., make sure we haven't torn it down while sleeping
> in a blocking send or receive). Perhaps it's not the best name?
How you'd hit that? Peer closing the socket while sleeping? Other
thread closing the socket wile sleeping? Both?
I think a state field in struct vsock_sock would be a better solution here.
>> What is *_allow?
>
> It's very basic filtering. We have specific addresses that we don't
> allow, and we look for them in the allow() functions. You can just
> return true if you like.
Can we make those calls optional too please?
>> What are all those notify_* calls?
>
> They're to do with signaling and flow-control. Again, they might
> not make any sense at all, but it's hard to know without having
> written another transport :)
I don't see a immediate need for them in the virto transport, but it's
hard to say without knowing what exactly they are doing.
>> Why do you need vsock_transport_{send,recv}_notify_data structs?
>> Can't this live in vsock_sock->trans?
>
> Those have to be setup on a per-call basis (per-thread), so it's
> just easier to have them on the stack of the send/recv calls. If
> you think there's a better name, or a better way to allocate them,
> I'm all ears.
Hmm. The struct content seems to be vmci-specific and it is in generic
code, which isn't that nice.
The notify_*_init could return a opaque pointer instead. But then
you'll need a notify_*_free too. And you can't place it on the stack
and thus have a allocation in the hot path, which I guess you are trying
to avoid in the first place.
No good idea offhand, but maybe that changes if the purpose of all those
notify_* calls is more clear.
cheers,
Gerd
^ 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