* Re: [PATCH net] ipv6: don't stop backtracking in fib6_lookup_1 if subtree does not match
From: Teco Boot @ 2013-08-09 14:12 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, yoshfuji, equinox, boutier
In-Reply-To: <20130807200637.GD32257@order.stressinduktion.org>
Double checked, it works. Tested with Common Open Research Emulator (CORE, from NRL).
=== But ===
I had some problems with CORE. Could be me, new kernel version, the patch, CORE using network namespaces, or combination of that. sudo hangs, reboot hangs, ip address show hangs.
With ps -ef, I see some suspected items:
root 4858 4161 0 15:57 ? 00:00:00 /sbin/ip -6 addr flush dev eth0
root 4859 2 0 15:57 ? 00:00:00 [kworker/0:3]
In the emulation I use, there was no IPv6.
I'll try to figure out if it is CORE & netns in combination with patch. Or just the patch. Or just the new kernel.
Just to speed up, any idea where to look for?
Thanks, Teco
Op 7 aug. 2013, om 22:06 heeft Hannes Frederic Sowa <hannes@stressinduktion.org> het volgende geschreven:
> On Wed, Aug 07, 2013 at 09:58:31PM +0200, Teco Boot wrote:
>> Seems to work!!
>>
>> babe has one entry less than cafe, this is correct.
>> Tomorrow I'll test with real traffic.
>
> Thanks for testing! I already used real traffic but it is better to
> double check.
>
> Greetings,
>
> Hannes
>
^ permalink raw reply
* Re: [PATCH net v2] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: Daniel Borkmann @ 2013-08-09 13:59 UTC (permalink / raw)
To: Vlad Yasevich; +Cc: davem, netdev, linux-sctp
In-Reply-To: <5204F55B.2040704@gmail.com>
On 08/09/2013 03:57 PM, Vlad Yasevich wrote:
> On 08/09/2013 09:27 AM, Daniel Borkmann wrote:
>> Probably this one is quite unlikely to be triggered, but it's more safe
>> to hold a pointer to asoc (instead of dereferencing), free the packet
>> chunks first, and access asoc though the pointer after we have called
>> sctp_transport_destroy_rcu() where the transport is being kfree()'d.
>> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
>> for deferred call_rcu's"). I also did the 8c98653f regression test and
>> it's fine that way.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>> v1->v2: do sctp_packet_free before call_rcu
>>
>> net/sctp/transport.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
>> index bdbbc3f..c691455 100644
>> --- a/net/sctp/transport.c
>> +++ b/net/sctp/transport.c
>> @@ -176,17 +176,19 @@ static void sctp_transport_destroy_rcu(struct rcu_head *head)
>> */
>> static void sctp_transport_destroy(struct sctp_transport *transport)
>> {
>> + struct sctp_association *asoc = transport->asoc;
>> +
>> if (unlikely(!transport->dead)) {
>> WARN(1, "Attempt to destroy undead transport %p!\n", transport);
>> return;
>> }
>>
>> - call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
>> -
>> sctp_packet_free(&transport->packet);
>>
>> - if (transport->asoc)
>> - sctp_association_put(transport->asoc);
>> + call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
>> +
>> + if (asoc != NULL)
>> + sctp_association_put(asoc);
>
> I think it is safe to move call_rcu to be the last call in this function. This should never be a last ref on the association. If
> by some chance it is, we'll get the same warning.
>
> And if you move call_rcu(), this becomes an ever smaller patch :)
Ok, that's also a way to go. Will send v3. :-)
> -vlad
>> }
>>
>> /* Start T3_rtx timer if it is not already running and update the heartbeat
>>
>
> --
> 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
^ permalink raw reply
* Re: linux-next: Tree for Aug 7
From: Phil Sutter @ 2013-08-09 13:58 UTC (permalink / raw)
To: David Miller
Cc: eric.dumazet, johannes, sedat.dilek, sfr, linux-next,
linux-kernel, netdev, hannes, linux-wireless, linville
In-Reply-To: <20130807.163621.84433966934449459.davem@davemloft.net>
On Wed, Aug 07, 2013 at 04:36:21PM -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 07 Aug 2013 16:27:48 -0700 (PDT)
>
> > Look, I'm going to fix this myself, because I'm pretty tired of
> > waiting for the obvious fix.
>
> Someone please test this:
>
> diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> index c623861..afc02a6 100644
> --- a/include/linux/etherdevice.h
> +++ b/include/linux/etherdevice.h
> @@ -29,6 +29,7 @@
>
> #ifdef __KERNEL__
> extern __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
> +extern __be16 __eth_type_trans(struct sk_buff *skb, struct net_device *dev);
> extern const struct header_ops eth_header_ops;
>
> extern int eth_header(struct sk_buff *skb, struct net_device *dev,
> diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
> index be1f64d..35dc1be 100644
> --- a/net/ethernet/eth.c
> +++ b/net/ethernet/eth.c
> @@ -146,6 +146,45 @@ int eth_rebuild_header(struct sk_buff *skb)
> EXPORT_SYMBOL(eth_rebuild_header);
>
> /**
> + * __eth_type_trans - only determine the packet's protocol ID.
> + * @skb: packet
> + * @dev: device
> + */
> +__be16 __eth_type_trans(struct sk_buff *skb, struct net_device *dev)
> +{
> + struct ethhdr *eth = (struct ethhdr *) skb->data;
> +
> + /*
> + * Some variants of DSA tagging don't have an ethertype field
> + * at all, so we check here whether one of those tagging
> + * variants has been configured on the receiving interface,
> + * and if so, set skb->protocol without looking at the packet.
> + */
> + if (netdev_uses_dsa_tags(dev))
> + return htons(ETH_P_DSA);
> + if (netdev_uses_trailer_tags(dev))
> + return htons(ETH_P_TRAILER);
> +
> + if (ntohs(eth->h_proto) >= ETH_P_802_3_MIN)
> + return eth->h_proto;
> +
> + /*
> + * This is a magic hack to spot IPX packets. Older Novell breaks
> + * the protocol design and runs IPX over 802.3 without an 802.2 LLC
> + * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
> + * won't work for fault tolerant netware but does for the rest.
> + */
> + if (skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)
> + return htons(ETH_P_802_3);
> +
> + /*
> + * Real 802.2 LLC
> + */
> + return htons(ETH_P_802_2);
> +}
> +EXPORT_SYMBOL(__eth_type_trans);
> +
> +/**
> * eth_type_trans - determine the packet's protocol ID.
> * @skb: received socket data
> * @dev: receiving network device
> @@ -184,33 +223,7 @@ __be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev)
> skb->pkt_type = PACKET_OTHERHOST;
> }
>
> - /*
> - * Some variants of DSA tagging don't have an ethertype field
> - * at all, so we check here whether one of those tagging
> - * variants has been configured on the receiving interface,
> - * and if so, set skb->protocol without looking at the packet.
> - */
> - if (netdev_uses_dsa_tags(dev))
> - return htons(ETH_P_DSA);
> - if (netdev_uses_trailer_tags(dev))
> - return htons(ETH_P_TRAILER);
> -
> - if (ntohs(eth->h_proto) >= ETH_P_802_3_MIN)
> - return eth->h_proto;
> -
> - /*
> - * This is a magic hack to spot IPX packets. Older Novell breaks
> - * the protocol design and runs IPX over 802.3 without an 802.2 LLC
> - * layer. We look for FFFF which isn't a used 802.2 SSAP/DSAP. This
> - * won't work for fault tolerant netware but does for the rest.
> - */
> - if (skb->len >= 2 && *(unsigned short *)(skb->data) == 0xFFFF)
> - return htons(ETH_P_802_3);
> -
> - /*
> - * Real 802.2 LLC
> - */
> - return htons(ETH_P_802_2);
> + return __eth_type_trans(skb, dev);
> }
> EXPORT_SYMBOL(eth_type_trans);
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 0c0f6c9..ec8e1c3 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -2003,7 +2003,7 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
> return err;
>
> if (dev->type == ARPHRD_ETHER)
> - skb->protocol = eth_type_trans(skb, dev);
> + skb->protocol = __eth_type_trans(skb, dev);
>
> data += dev->hard_header_len;
> to_write -= dev->hard_header_len;
> @@ -2332,13 +2332,13 @@ static int packet_snd(struct socket *sock,
> sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
>
> if (dev->type == ARPHRD_ETHER) {
> - skb->protocol = eth_type_trans(skb, dev);
> + skb->protocol = __eth_type_trans(skb, dev);
> if (skb->protocol == htons(ETH_P_8021Q))
> reserve += VLAN_HLEN;
> } else {
> skb->protocol = proto;
> - skb->dev = dev;
> }
> + skb->dev = dev;
>
> if (!gso_type && (len > dev->mtu + reserve + extra_len)) {
> err = -EMSGSIZE;
The problem with this patch is __eth_type_trans() assuming the MAC
header at skb->data which might be correct in the most cases, but not
when called from eth_type_trans() as the later sets skb->data to after
the ethernet header (which was the problem from the beginning).
Best wishes, Phil
^ permalink raw reply
* Re: [PATCH net v2] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: Vlad Yasevich @ 2013-08-09 13:57 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
In-Reply-To: <1376054842-376-1-git-send-email-dborkman@redhat.com>
On 08/09/2013 09:27 AM, Daniel Borkmann wrote:
> Probably this one is quite unlikely to be triggered, but it's more safe
> to hold a pointer to asoc (instead of dereferencing), free the packet
> chunks first, and access asoc though the pointer after we have called
> sctp_transport_destroy_rcu() where the transport is being kfree()'d.
> Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
> for deferred call_rcu's"). I also did the 8c98653f regression test and
> it's fine that way.
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---
> v1->v2: do sctp_packet_free before call_rcu
>
> net/sctp/transport.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index bdbbc3f..c691455 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -176,17 +176,19 @@ static void sctp_transport_destroy_rcu(struct rcu_head *head)
> */
> static void sctp_transport_destroy(struct sctp_transport *transport)
> {
> + struct sctp_association *asoc = transport->asoc;
> +
> if (unlikely(!transport->dead)) {
> WARN(1, "Attempt to destroy undead transport %p!\n", transport);
> return;
> }
>
> - call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> -
> sctp_packet_free(&transport->packet);
>
> - if (transport->asoc)
> - sctp_association_put(transport->asoc);
> + call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
> +
> + if (asoc != NULL)
> + sctp_association_put(asoc);
I think it is safe to move call_rcu to be the last call in this
function. This should never be a last ref on the association. If
by some chance it is, we'll get the same warning.
And if you move call_rcu(), this becomes an ever smaller patch :)
-vlad
> }
>
> /* Start T3_rtx timer if it is not already running and update the heartbeat
>
^ permalink raw reply
* Re: [PATCH RFC 0/5] netfilter: implement netfilter SYN proxy
From: Neal Cardwell @ 2013-08-09 13:55 UTC (permalink / raw)
To: Patrick McHardy
Cc: David Miller, Eric Dumazet, pablo, netfilter-devel, Netdev, mph,
jesper.brouer, as
In-Reply-To: <20130808001344.GA24450@macbook.localnet>
> It seems, Windows stopped using tcp timestamps at least in windows 8 by
> default.
I agree we should not let the Windows TCP stack behavior dictate the
approach, but just wanted to point out that it's not just Windows 8
that lacks timestamps. It's more that Windows 7 is about the only
sizable Windows deployment that does use timestamps. Windows NT,
Windows 2000, Windows XP, Windows Vista, and Windows 8 all do not use
timestamps by default, and that adds up to about 30-40% of Internet
clients, depending on who's measuring. Something to keep in mind.
neal
^ permalink raw reply
* Re: [PATCH net] net: sctp: sctp_assoc_control_transport: fix MTU size in SCTP_PF state
From: Vlad Yasevich @ 2013-08-09 13:39 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, kheiss, netdev, linux-sctp, Neil Horman
In-Reply-To: <1376053748-31783-1-git-send-email-dborkman@redhat.com>
On 08/09/2013 09:09 AM, Daniel Borkmann wrote:
> The SCTP Quick failover draft [1] section 5.1, point 5 says that the cwnd
> should be 1 MTU. So, instead of 1, set it to 1 MTU.
>
> [1] https://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05
>
> Reported-by: Karl Heiss <kheiss@gmail.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
> ---
> net/sctp/associola.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index bce5b79..ab67efc 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -846,12 +846,12 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
> else
> spc_state = SCTP_ADDR_AVAILABLE;
> /* Don't inform ULP about transition from PF to
> - * active state and set cwnd to 1, see SCTP
> + * active state and set cwnd to 1 MTU, see SCTP
> * Quick failover draft section 5.1, point 5
> */
> if (transport->state == SCTP_PF) {
> ulp_notify = false;
> - transport->cwnd = 1;
> + transport->cwnd = asoc->pathmtu;
> }
> transport->state = SCTP_ACTIVE;
> break;
>
Acked-by: Vlad Yasevich <vyasevich@gmail.com>
-vlad
^ permalink raw reply
* [PATCH net v2] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: Daniel Borkmann @ 2013-08-09 13:27 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Probably this one is quite unlikely to be triggered, but it's more safe
to hold a pointer to asoc (instead of dereferencing), free the packet
chunks first, and access asoc though the pointer after we have called
sctp_transport_destroy_rcu() where the transport is being kfree()'d.
Introduced by commit 8c98653f ("sctp: sctp_close: fix release of bindings
for deferred call_rcu's"). I also did the 8c98653f regression test and
it's fine that way.
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
v1->v2: do sctp_packet_free before call_rcu
net/sctp/transport.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index bdbbc3f..c691455 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -176,17 +176,19 @@ static void sctp_transport_destroy_rcu(struct rcu_head *head)
*/
static void sctp_transport_destroy(struct sctp_transport *transport)
{
+ struct sctp_association *asoc = transport->asoc;
+
if (unlikely(!transport->dead)) {
WARN(1, "Attempt to destroy undead transport %p!\n", transport);
return;
}
- call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
-
sctp_packet_free(&transport->packet);
- if (transport->asoc)
- sctp_association_put(transport->asoc);
+ call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
+
+ if (asoc != NULL)
+ sctp_association_put(asoc);
}
/* Start T3_rtx timer if it is not already running and update the heartbeat
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH] net:ethernet:korina.c Removed IRQF_DISABLED
From: Peter Senna Tschudin @ 2013-08-09 13:15 UTC (permalink / raw)
To: Kumar Gaurav
Cc: Joe Perches, David Miller, Dan Carpenter, emilio, mugunthanvnm,
jg1.han, hsweeten, netdev, kernel-janitors
In-Reply-To: <5204641C.3090905@gmail.com>
Hello Kumar,
Coccinelle (http://coccinelle.lip6.fr/) can help you in this task. The
semantic patch below takes care of both cases for you:
@@
expression e;
@@
- IRQF_DISABLED | e
+ e
@@
@@
-IRQF_DISABLED
+0
When running it on drivers net, Coccinelle found:
diff -u -p a/hamradio/scc.c b/hamradio/scc.c
--- a/hamradio/scc.c
+++ b/hamradio/scc.c
@@ -1734,7 +1734,7 @@ static int scc_net_ioctl(struct net_devi
if (!Ivec[hwcfg.irq].used && hwcfg.irq)
{
if (request_irq(hwcfg.irq, scc_isr,
- IRQF_DISABLED, "AX.25 SCC",
+ 0, "AX.25 SCC",
(void *)(long) hwcfg.irq))
printk(KERN_WARNING "z8530drv: warning, cannot get IRQ %d\n", hwcfg.irq);
else
diff -u -p a/hamradio/baycom_ser_hdx.c b/hamradio/baycom_ser_hdx.c
--- a/hamradio/baycom_ser_hdx.c
+++ b/hamradio/baycom_ser_hdx.c
@@ -490,7 +490,7 @@ static int ser12_open(struct net_device
outb(0, FCR(dev->base_addr)); /* disable FIFOs */
outb(0x0d, MCR(dev->base_addr));
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
+ if (request_irq(dev->irq, ser12_interrupt, IRQF_SHARED,
"baycom_ser12", dev)) {
release_region(dev->base_addr, SER12_EXTENT);
return -EBUSY;
diff -u -p a/hamradio/baycom_ser_fdx.c b/hamradio/baycom_ser_fdx.c
--- a/hamradio/baycom_ser_fdx.c
+++ b/hamradio/baycom_ser_fdx.c
@@ -445,7 +445,7 @@ static int ser12_open(struct net_device
outb(0, FCR(dev->base_addr)); /* disable FIFOs */
outb(0x0d, MCR(dev->base_addr));
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, ser12_interrupt, IRQF_DISABLED | IRQF_SHARED,
+ if (request_irq(dev->irq, ser12_interrupt, IRQF_SHARED,
"baycom_ser_fdx", dev)) {
release_region(dev->base_addr, SER12_EXTENT);
return -EBUSY;
diff -u -p a/hamradio/yam.c b/hamradio/yam.c
--- a/hamradio/yam.c
+++ b/hamradio/yam.c
@@ -888,7 +888,7 @@ static int yam_open(struct net_device *d
goto out_release_base;
}
outb(0, IER(dev->base_addr));
- if (request_irq(dev->irq, yam_interrupt, IRQF_DISABLED |
IRQF_SHARED, dev->name, dev)) {
+ if (request_irq(dev->irq, yam_interrupt, IRQF_SHARED, dev->name, dev)) {
printk(KERN_ERR "%s: irq %d busy\n", dev->name, dev->irq);
ret = -EBUSY;
goto out_release_base;
diff -u -p a/wireless/p54/p54spi.c b/wireless/p54/p54spi.c
--- a/wireless/p54/p54spi.c
+++ b/wireless/p54/p54spi.c
@@ -636,7 +636,7 @@ static int p54spi_probe(struct spi_devic
gpio_direction_input(p54spi_gpio_irq);
ret = request_irq(gpio_to_irq(p54spi_gpio_irq),
- p54spi_interrupt, IRQF_DISABLED, "p54spi",
+ p54spi_interrupt, 0, "p54spi",
priv->spi);
if (ret < 0) {
dev_err(&priv->spi->dev, "request_irq() failed");
diff -u -p a/wan/hostess_sv11.c b/wan/hostess_sv11.c
--- a/wan/hostess_sv11.c
+++ b/wan/hostess_sv11.c
@@ -220,7 +220,7 @@ static struct z8530_dev *sv11_init(int i
/* We want a fast IRQ for this device. Actually we'd like an even faster
IRQ ;) - This is one driver RtLinux is made for */
- if (request_irq(irq, z8530_interrupt, IRQF_DISABLED,
+ if (request_irq(irq, z8530_interrupt, 0,
"Hostess SV11", sv) < 0) {
pr_warn("IRQ %d already in use\n", irq);
goto err_irq;
diff -u -p a/wan/sealevel.c b/wan/sealevel.c
--- a/wan/sealevel.c
+++ b/wan/sealevel.c
@@ -266,7 +266,7 @@ static __init struct slvl_board *slvl_in
/* We want a fast IRQ for this device. Actually we'd like an even faster
IRQ ;) - This is one driver RtLinux is made for */
- if (request_irq(irq, z8530_interrupt, IRQF_DISABLED,
+ if (request_irq(irq, z8530_interrupt, 0,
"SeaLevel", dev) < 0) {
pr_warn("IRQ %d already in use\n", irq);
goto err_request_irq;
diff -u -p a/ethernet/natsemi/xtsonic.c b/ethernet/natsemi/xtsonic.c
--- a/ethernet/natsemi/xtsonic.c
+++ b/ethernet/natsemi/xtsonic.c
@@ -95,7 +95,7 @@ static int xtsonic_open(struct net_devic
{
int retval;
- retval = request_irq(dev->irq, sonic_interrupt, IRQF_DISABLED,
+ retval = request_irq(dev->irq, sonic_interrupt, 0,
"sonic", dev);
if (retval) {
printk(KERN_ERR "%s: unable to get IRQ %d.\n",
diff -u -p a/ethernet/natsemi/jazzsonic.c b/ethernet/natsemi/jazzsonic.c
--- a/ethernet/natsemi/jazzsonic.c
+++ b/ethernet/natsemi/jazzsonic.c
@@ -83,7 +83,7 @@ static int jazzsonic_open(struct net_dev
{
int retval;
- retval = request_irq(dev->irq, sonic_interrupt, IRQF_DISABLED,
+ retval = request_irq(dev->irq, sonic_interrupt, 0,
"sonic", dev);
if (retval) {
printk(KERN_ERR "%s: unable to get IRQ %d.\n",
diff -u -p a/ethernet/smsc/smsc9420.c b/ethernet/smsc/smsc9420.c
--- a/ethernet/smsc/smsc9420.c
+++ b/ethernet/smsc/smsc9420.c
@@ -1356,7 +1356,7 @@ static int smsc9420_open(struct net_devi
smsc9420_reg_write(pd, INT_STAT, 0xFFFFFFFF);
smsc9420_pci_flush_write(pd);
- result = request_irq(irq, smsc9420_isr, IRQF_SHARED | IRQF_DISABLED,
+ result = request_irq(irq, smsc9420_isr, IRQF_SHARED,
DRV_NAME, pd);
if (result) {
smsc_warn(IFUP, "Unable to use IRQ = %d", irq);
diff -u -p a/ethernet/marvell/pxa168_eth.c b/ethernet/marvell/pxa168_eth.c
--- a/ethernet/marvell/pxa168_eth.c
+++ b/ethernet/marvell/pxa168_eth.c
@@ -1125,7 +1125,7 @@ static int pxa168_eth_open(struct net_de
int err;
err = request_irq(dev->irq, pxa168_eth_int_handler,
- IRQF_DISABLED, dev->name, dev);
+ 0, dev->name, dev);
if (err) {
dev_err(&dev->dev, "can't assign irq\n");
return -EAGAIN;
diff -u -p a/ethernet/pasemi/pasemi_mac.c b/ethernet/pasemi/pasemi_mac.c
--- a/ethernet/pasemi/pasemi_mac.c
+++ b/ethernet/pasemi/pasemi_mac.c
@@ -1220,7 +1220,7 @@ static int pasemi_mac_open(struct net_de
snprintf(mac->tx_irq_name, sizeof(mac->tx_irq_name), "%s tx",
dev->name);
- ret = request_irq(mac->tx->chan.irq, pasemi_mac_tx_intr, IRQF_DISABLED,
+ ret = request_irq(mac->tx->chan.irq, pasemi_mac_tx_intr, 0,
mac->tx_irq_name, mac->tx);
if (ret) {
dev_err(&mac->pdev->dev, "request_irq of irq %d failed: %d\n",
@@ -1231,7 +1231,7 @@ static int pasemi_mac_open(struct net_de
snprintf(mac->rx_irq_name, sizeof(mac->rx_irq_name), "%s rx",
dev->name);
- ret = request_irq(mac->rx->chan.irq, pasemi_mac_rx_intr, IRQF_DISABLED,
+ ret = request_irq(mac->rx->chan.irq, pasemi_mac_rx_intr, 0,
mac->rx_irq_name, mac->rx);
if (ret) {
dev_err(&mac->pdev->dev, "request_irq of irq %d failed: %d\n",
diff -u -p a/ethernet/ibm/ehea/ehea_main.c b/ethernet/ibm/ehea/ehea_main.c
--- a/ethernet/ibm/ehea/ehea_main.c
+++ b/ethernet/ibm/ehea/ehea_main.c
@@ -1297,7 +1297,7 @@ static int ehea_reg_interrupts(struct ne
ret = ibmebus_request_irq(port->qp_eq->attr.ist1,
ehea_qp_aff_irq_handler,
- IRQF_DISABLED, port->int_aff_name, port);
+ 0, port->int_aff_name, port);
if (ret) {
netdev_err(dev, "failed registering irq for qp_aff_irq_handler:ist=%X\n",
port->qp_eq->attr.ist1);
@@ -1315,7 +1315,7 @@ static int ehea_reg_interrupts(struct ne
"%s-queue%d", dev->name, i);
ret = ibmebus_request_irq(pr->eq->attr.ist1,
ehea_recv_irq_handler,
- IRQF_DISABLED, pr->int_send_name,
+ 0, pr->int_send_name,
pr);
if (ret) {
netdev_err(dev, "failed registering irq for ehea_queue
port_res_nr:%d, ist=%X\n",
@@ -3332,7 +3332,7 @@ static int ehea_probe_adapter(struct pla
}
ret = ibmebus_request_irq(adapter->neq->attr.ist1,
- ehea_interrupt_neq, IRQF_DISABLED,
+ ehea_interrupt_neq, 0,
"ehea_neq", adapter);
if (ret) {
dev_err(&dev->dev, "requesting NEQ IRQ failed\n");
diff -u -p a/ethernet/toshiba/ps3_gelic_net.c b/ethernet/toshiba/ps3_gelic_net.c
--- a/ethernet/toshiba/ps3_gelic_net.c
+++ b/ethernet/toshiba/ps3_gelic_net.c
@@ -1727,7 +1727,7 @@ static int ps3_gelic_driver_probe(struct
goto fail_alloc_irq;
}
result = request_irq(card->irq, gelic_card_interrupt,
- IRQF_DISABLED, netdev->name, card);
+ 0, netdev->name, card);
if (result) {
dev_info(ctodev(card), "%s:request_irq failed (%d)\n",
diff -u -p a/ethernet/lantiq_etop.c b/ethernet/lantiq_etop.c
--- a/ethernet/lantiq_etop.c
+++ b/ethernet/lantiq_etop.c
@@ -282,7 +282,7 @@ ltq_etop_hw_init(struct net_device *dev)
if (IS_TX(i)) {
ltq_dma_alloc_tx(&ch->dma);
- request_irq(irq, ltq_etop_dma_irq, IRQF_DISABLED,
+ request_irq(irq, ltq_etop_dma_irq, 0,
"etop_tx", priv);
} else if (IS_RX(i)) {
ltq_dma_alloc_rx(&ch->dma);
@@ -291,7 +291,7 @@ ltq_etop_hw_init(struct net_device *dev)
if (ltq_etop_alloc_skb(ch))
return -ENOMEM;
ch->dma.desc = 0;
- request_irq(irq, ltq_etop_dma_irq, IRQF_DISABLED,
+ request_irq(irq, ltq_etop_dma_irq, 0,
"etop_rx", priv);
}
ch->dma.irq = irq;
diff -u -p a/ethernet/hp/hp100.c b/ethernet/hp/hp100.c
--- a/ethernet/hp/hp100.c
+++ b/ethernet/hp/hp100.c
@@ -1097,7 +1097,7 @@ static int hp100_open(struct net_device
/* New: if bus is PCI or EISA, interrupts might be shared interrupts */
if (request_irq(dev->irq, hp100_interrupt,
lp->bus == HP100_BUS_PCI || lp->bus ==
- HP100_BUS_EISA ? IRQF_SHARED : IRQF_DISABLED,
+ HP100_BUS_EISA ? IRQF_SHARED : 0,
"hp100", dev)) {
printk("hp100: %s: unable to get IRQ %d\n", dev->name, dev->irq);
return -EAGAIN;
diff -u -p a/ethernet/broadcom/bcm63xx_enet.c b/ethernet/broadcom/bcm63xx_enet.c
--- a/ethernet/broadcom/bcm63xx_enet.c
+++ b/ethernet/broadcom/bcm63xx_enet.c
@@ -926,13 +926,13 @@ static int bcm_enet_open(struct net_devi
if (ret)
goto out_phy_disconnect;
- ret = request_irq(priv->irq_rx, bcm_enet_isr_dma, IRQF_DISABLED,
+ ret = request_irq(priv->irq_rx, bcm_enet_isr_dma, 0,
dev->name, dev);
if (ret)
goto out_freeirq;
ret = request_irq(priv->irq_tx, bcm_enet_isr_dma,
- IRQF_DISABLED, dev->name, dev);
+ 0, dev->name, dev);
if (ret)
goto out_freeirq_rx;
@@ -2158,13 +2158,13 @@ static int bcm_enetsw_open(struct net_de
enet_dmac_writel(priv, 0, ENETDMAC_IRMASK, priv->tx_chan);
ret = request_irq(priv->irq_rx, bcm_enet_isr_dma,
- IRQF_DISABLED, dev->name, dev);
+ 0, dev->name, dev);
if (ret)
goto out_freeirq;
if (priv->irq_tx != -1) {
ret = request_irq(priv->irq_tx, bcm_enet_isr_dma,
- IRQF_DISABLED, dev->name, dev);
+ 0, dev->name, dev);
if (ret)
goto out_freeirq_rx;
}
diff -u -p a/ethernet/korina.c b/ethernet/korina.c
--- a/ethernet/korina.c
+++ b/ethernet/korina.c
@@ -996,14 +996,14 @@ static int korina_open(struct net_device
* that handles the Done Finished
* Ovr and Und Events */
ret = request_irq(lp->rx_irq, korina_rx_dma_interrupt,
- IRQF_DISABLED, "Korina ethernet Rx", dev);
+ 0, "Korina ethernet Rx", dev);
if (ret < 0) {
printk(KERN_ERR "%s: unable to get Rx DMA IRQ %d\n",
dev->name, lp->rx_irq);
goto err_release;
}
ret = request_irq(lp->tx_irq, korina_tx_dma_interrupt,
- IRQF_DISABLED, "Korina ethernet Tx", dev);
+ 0, "Korina ethernet Tx", dev);
if (ret < 0) {
printk(KERN_ERR "%s: unable to get Tx DMA IRQ %d\n",
dev->name, lp->tx_irq);
@@ -1012,7 +1012,7 @@ static int korina_open(struct net_device
/* Install handler for overrun error. */
ret = request_irq(lp->ovr_irq, korina_ovr_interrupt,
- IRQF_DISABLED, "Ethernet Overflow", dev);
+ 0, "Ethernet Overflow", dev);
if (ret < 0) {
printk(KERN_ERR "%s: unable to get OVR IRQ %d\n",
dev->name, lp->ovr_irq);
@@ -1021,7 +1021,7 @@ static int korina_open(struct net_device
/* Install handler for underflow error. */
ret = request_irq(lp->und_irq, korina_und_interrupt,
- IRQF_DISABLED, "Ethernet Underflow", dev);
+ 0, "Ethernet Underflow", dev);
if (ret < 0) {
printk(KERN_ERR "%s: unable to get UND IRQ %d\n",
dev->name, lp->und_irq);
diff -u -p a/ethernet/amd/sun3lance.c b/ethernet/amd/sun3lance.c
--- a/ethernet/amd/sun3lance.c
+++ b/ethernet/amd/sun3lance.c
@@ -358,7 +358,7 @@ static int __init lance_probe( struct ne
REGA(CSR0) = CSR0_STOP;
- if (request_irq(LANCE_IRQ, lance_interrupt, IRQF_DISABLED, "SUN3
Lance", dev) < 0) {
+ if (request_irq(LANCE_IRQ, lance_interrupt, 0, "SUN3 Lance", dev) < 0) {
#ifdef CONFIG_SUN3
iounmap((void __iomem *)ioaddr);
#endif
diff -u -p a/ethernet/dec/tulip/de4x5.c b/ethernet/dec/tulip/de4x5.c
--- a/ethernet/dec/tulip/de4x5.c
+++ b/ethernet/dec/tulip/de4x5.c
@@ -1321,7 +1321,7 @@ de4x5_open(struct net_device *dev)
if (request_irq(dev->irq, de4x5_interrupt, IRQF_SHARED,
lp->adapter_name, dev)) {
printk("de4x5_open(): Requested IRQ%d is busy - attemping
FAST/SHARE...", dev->irq);
- if (request_irq(dev->irq, de4x5_interrupt, IRQF_DISABLED | IRQF_SHARED,
+ if (request_irq(dev->irq, de4x5_interrupt, IRQF_SHARED,
lp->adapter_name, dev)) {
printk("\n Cannot get IRQ- reconfigure your hardware.\n");
disable_ast(dev);
diff -u -p a/ethernet/freescale/fec_main.c b/ethernet/freescale/fec_main.c
--- a/ethernet/freescale/fec_main.c
+++ b/ethernet/freescale/fec_main.c
@@ -2173,7 +2173,7 @@ fec_probe(struct platform_device *pdev)
goto failed_irq;
}
ret = devm_request_irq(&pdev->dev, irq, fec_enet_interrupt,
- IRQF_DISABLED, pdev->name, ndev);
+ 0, pdev->name, ndev);
if (ret)
goto failed_irq;
}
diff -u -p a/ethernet/adi/bfin_mac.c b/ethernet/adi/bfin_mac.c
--- a/ethernet/adi/bfin_mac.c
+++ b/ethernet/adi/bfin_mac.c
@@ -530,7 +530,7 @@ static int bfin_mac_ethtool_setwol(struc
if (lp->wol && !lp->irq_wake_requested) {
/* register wake irq handler */
rc = request_irq(IRQ_MAC_WAKEDET, bfin_mac_wake_interrupt,
- IRQF_DISABLED, "EMAC_WAKE", dev);
+ 0, "EMAC_WAKE", dev);
if (rc)
return rc;
lp->irq_wake_requested = true;
@@ -1686,7 +1686,7 @@ static int bfin_mac_probe(struct platfor
/* now, enable interrupts */
/* register irq handler */
rc = request_irq(IRQ_MAC_RX, bfin_mac_interrupt,
- IRQF_DISABLED, "EMAC_RX", ndev);
+ 0, "EMAC_RX", ndev);
if (rc) {
dev_err(&pdev->dev, "Cannot request Blackfin MAC RX IRQ!\n");
rc = -EBUSY;
diff -u -p a/irda/bfin_sir.c b/irda/bfin_sir.c
--- a/irda/bfin_sir.c
+++ b/irda/bfin_sir.c
@@ -411,12 +411,12 @@ static int bfin_sir_startup(struct bfin_
#else
- if (request_irq(port->irq, bfin_sir_rx_int, IRQF_DISABLED,
"BFIN_SIR_RX", dev)) {
+ if (request_irq(port->irq, bfin_sir_rx_int, 0, "BFIN_SIR_RX", dev)) {
dev_warn(&dev->dev, "Unable to attach SIR RX interrupt\n");
return -EBUSY;
}
- if (request_irq(port->irq+1, bfin_sir_tx_int, IRQF_DISABLED,
"BFIN_SIR_TX", dev)) {
+ if (request_irq(port->irq+1, bfin_sir_tx_int, 0, "BFIN_SIR_TX", dev)) {
dev_warn(&dev->dev, "Unable to attach SIR TX interrupt\n");
free_irq(port->irq, dev);
return -EBUSY;
diff -u -p a/irda/sh_irda.c b/irda/sh_irda.c
--- a/irda/sh_irda.c
+++ b/irda/sh_irda.c
@@ -804,7 +804,7 @@ static int sh_irda_probe(struct platform
goto err_mem_4;
platform_set_drvdata(pdev, ndev);
- err = request_irq(irq, sh_irda_irq, IRQF_DISABLED, "sh_irda", self);
+ err = request_irq(irq, sh_irda_irq, 0, "sh_irda", self);
if (err) {
dev_warn(&pdev->dev, "Unable to attach sh_irda interrupt\n");
goto err_mem_4;
diff -u -p a/irda/sh_sir.c b/irda/sh_sir.c
--- a/irda/sh_sir.c
+++ b/irda/sh_sir.c
@@ -761,7 +761,7 @@ static int sh_sir_probe(struct platform_
goto err_mem_4;
platform_set_drvdata(pdev, ndev);
- err = request_irq(irq, sh_sir_irq, IRQF_DISABLED, "sh_sir", self);
+ err = request_irq(irq, sh_sir_irq, 0, "sh_sir", self);
if (err) {
dev_warn(&pdev->dev, "Unable to attach sh_sir interrupt\n");
goto err_mem_4;
diff -u -p a/irda/donauboe.c b/irda/donauboe.c
--- a/irda/donauboe.c
+++ b/irda/donauboe.c
@@ -1352,7 +1352,7 @@ toshoboe_net_open (struct net_device *de
return 0;
rc = request_irq (self->io.irq, toshoboe_interrupt,
- IRQF_SHARED | IRQF_DISABLED, dev->name, self);
+ IRQF_SHARED, dev->name, self);
if (rc)
return rc;
@@ -1559,7 +1559,7 @@ toshoboe_open (struct pci_dev *pci_dev,
self->io.fir_base = self->base;
self->io.fir_ext = OBOE_IO_EXTENT;
self->io.irq = pci_dev->irq;
- self->io.irqflags = IRQF_SHARED | IRQF_DISABLED;
+ self->io.irqflags = IRQF_SHARED;
self->speed = self->io.speed = 9600;
self->async = 0;
On Fri, Aug 9, 2013 at 5:38 AM, Kumar Gaurav
<kumargauravgupta3@gmail.com> wrote:
> On Friday 09 August 2013 08:58 AM, Joe Perches wrote:
>>
>> On Fri, 2013-08-09 at 08:30 +0530, Kumar Gaurav wrote:
>>>
>>> On Friday 09 August 2013 08:24 AM, Joe Perches wrote:
>>>>
>>>> On Fri, 2013-08-09 at 08:15 +0530, Kumar Gaurav wrote:
>>>>>
>>>>> On Friday 09 August 2013 08:09 AM, Joe Perches wrote:
>>>>
>>>> []
>>>>>>
>>>>>> And, maybe it'd be better to use IRQF_TRIGGER_NONE
>>>>>> instead of 0.
>>>>>
>>>>> I tried googling what to replace IRQF_DISABLED with but found nothing.
>>>>> In the patch fixed earlier (not by me) it was replaced with 0 so i did
>>>>> same. But from now on I'll use IRQF_TRIGGER_NONE. Thanks
>>>>
>>>> Maybe that's not the right thing to do.
>>>> 0 is what's almost exclusively used.
>>>> IRQF_TRIGGER_NONE is only used a few times.
>>>> It's also a lot longer.
>>>>
>>> Sorry to poke back. But just want to confirm. I should use
>>> IRQF_TRIGGER_NONE and not 0 right?
>>
>> Hi again Kumar.
>>
>> There's no rush to submit these sorts of patches.
>> Nothing is going to really be improved because of these
>> changes.
>>
>> Submit an overall rfc patch description to the various
>> mailing lists (affected MAINTAINERS generally don't read
>> lkml but do read their specific mailing list) with options
>> like 0 or IRQF_TRIGGER_NONE to show what the issues are
>> and why you're doing this.
>>
>> Wait at least a week for any comment.
>>
>> You'll likely get some "useless churn" emails.
>> You can generally ignore those.
>>
>> If you get any real comments, deal with them.
>>
>> Then after at least that week passes, submit the patches.
>>
>> cheers, Joe
>>
> Will surely follow that thanks :)
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Peter
^ permalink raw reply
* [PATCH net] net: sctp: sctp_assoc_control_transport: fix MTU size in SCTP_PF state
From: Daniel Borkmann @ 2013-08-09 13:09 UTC (permalink / raw)
To: davem; +Cc: kheiss, netdev, linux-sctp, Neil Horman
The SCTP Quick failover draft [1] section 5.1, point 5 says that the cwnd
should be 1 MTU. So, instead of 1, set it to 1 MTU.
[1] https://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05
Reported-by: Karl Heiss <kheiss@gmail.com>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Neil Horman <nhorman@tuxdriver.com>
---
net/sctp/associola.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index bce5b79..ab67efc 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -846,12 +846,12 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
else
spc_state = SCTP_ADDR_AVAILABLE;
/* Don't inform ULP about transition from PF to
- * active state and set cwnd to 1, see SCTP
+ * active state and set cwnd to 1 MTU, see SCTP
* Quick failover draft section 5.1, point 5
*/
if (transport->state == SCTP_PF) {
ulp_notify = false;
- transport->cwnd = 1;
+ transport->cwnd = asoc->pathmtu;
}
transport->state = SCTP_ACTIVE;
break;
--
1.7.11.7
^ permalink raw reply related
* [PATCH] stmmac: fix init_dma_desc_rings() to handle errors
From: Bartlomiej Zolnierkiewicz @ 2013-08-09 12:02 UTC (permalink / raw)
To: Giuseppe Cavallaro; +Cc: netdev
In stmmac_init_rx_buffers():
* add missing handling of dma_map_single() error
* remove superfluous unlikely() optimization while at it
Add stmmac_free_rx_buffers() helper and use it in dma_free_rx_skbufs().
In init_dma_desc_rings():
* add missing handling of kmalloc_array() errors
* fix handling of dma_alloc_coherent() and stmmac_init_rx_buffers() errors
* make function return an error value on error and 0 on success
In stmmac_open():
* add handling of init_dma_desc_rings() return value
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 111 ++++++++++++++++++----
1 file changed, 92 insertions(+), 19 deletions(-)
Index: b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
===================================================================
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 2013-08-08 14:36:44.000000000 +0200
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c 2013-08-08 15:31:27.015438117 +0200
@@ -939,15 +939,20 @@ static int stmmac_init_rx_buffers(struct
skb = __netdev_alloc_skb(priv->dev, priv->dma_buf_sz + NET_IP_ALIGN,
GFP_KERNEL);
- if (unlikely(skb == NULL)) {
+ if (!skb) {
pr_err("%s: Rx init fails; skb is NULL\n", __func__);
- return 1;
+ return -ENOMEM;
}
skb_reserve(skb, NET_IP_ALIGN);
priv->rx_skbuff[i] = skb;
priv->rx_skbuff_dma[i] = dma_map_single(priv->device, skb->data,
priv->dma_buf_sz,
DMA_FROM_DEVICE);
+ if (dma_mapping_error(priv->device, priv->rx_skbuff_dma[i])) {
+ pr_err("%s: DMA mapping error\n", __func__);
+ dev_kfree_skb_any(skb);
+ return -EINVAL;
+ }
p->des2 = priv->rx_skbuff_dma[i];
@@ -958,6 +963,16 @@ static int stmmac_init_rx_buffers(struct
return 0;
}
+static void stmmac_free_rx_buffers(struct stmmac_priv *priv, int i)
+{
+ if (priv->rx_skbuff[i]) {
+ dma_unmap_single(priv->device, priv->rx_skbuff_dma[i],
+ priv->dma_buf_sz, DMA_FROM_DEVICE);
+ dev_kfree_skb_any(priv->rx_skbuff[i]);
+ }
+ priv->rx_skbuff[i] = NULL;
+}
+
/**
* init_dma_desc_rings - init the RX/TX descriptor rings
* @dev: net device structure
@@ -965,13 +980,14 @@ static int stmmac_init_rx_buffers(struct
* and allocates the socket buffers. It suppors the chained and ring
* modes.
*/
-static void init_dma_desc_rings(struct net_device *dev)
+static int init_dma_desc_rings(struct net_device *dev)
{
int i;
struct stmmac_priv *priv = netdev_priv(dev);
unsigned int txsize = priv->dma_tx_size;
unsigned int rxsize = priv->dma_rx_size;
unsigned int bfsize = 0;
+ int ret = -ENOMEM;
/* Set the max buffer size according to the DESC mode
* and the MTU. Note that RING mode allows 16KiB bsize.
@@ -992,34 +1008,60 @@ static void init_dma_desc_rings(struct n
dma_extended_desc),
&priv->dma_rx_phy,
GFP_KERNEL);
+ if (!priv->dma_erx)
+ goto err_dma;
+
priv->dma_etx = dma_alloc_coherent(priv->device, txsize *
sizeof(struct
dma_extended_desc),
&priv->dma_tx_phy,
GFP_KERNEL);
- if ((!priv->dma_erx) || (!priv->dma_etx))
- return;
+ if (!priv->dma_etx) {
+ dma_free_coherent(priv->device, priv->dma_rx_size *
+ sizeof(struct dma_extended_desc),
+ priv->dma_erx, priv->dma_rx_phy);
+ goto err_dma;
+ }
} else {
priv->dma_rx = dma_alloc_coherent(priv->device, rxsize *
sizeof(struct dma_desc),
&priv->dma_rx_phy,
GFP_KERNEL);
+ if (!priv->dma_rx)
+ goto err_dma;
+
priv->dma_tx = dma_alloc_coherent(priv->device, txsize *
sizeof(struct dma_desc),
&priv->dma_tx_phy,
GFP_KERNEL);
- if ((!priv->dma_rx) || (!priv->dma_tx))
- return;
+ if (!priv->dma_tx) {
+ dma_free_coherent(priv->device, priv->dma_rx_size *
+ sizeof(struct dma_desc),
+ priv->dma_rx, priv->dma_rx_phy);
+ goto err_dma;
+ }
}
priv->rx_skbuff_dma = kmalloc_array(rxsize, sizeof(dma_addr_t),
GFP_KERNEL);
+ if (!priv->rx_skbuff_dma)
+ goto err_rx_skbuff_dma;
+
priv->rx_skbuff = kmalloc_array(rxsize, sizeof(struct sk_buff *),
GFP_KERNEL);
+ if (!priv->rx_skbuff)
+ goto err_rx_skbuff;
+
priv->tx_skbuff_dma = kmalloc_array(txsize, sizeof(dma_addr_t),
GFP_KERNEL);
+ if (!priv->tx_skbuff_dma)
+ goto err_tx_skbuff_dma;
+
priv->tx_skbuff = kmalloc_array(txsize, sizeof(struct sk_buff *),
GFP_KERNEL);
+ if (!priv->tx_skbuff)
+ goto err_tx_skbuff;
+
if (netif_msg_probe(priv)) {
pr_debug("(%s) dma_rx_phy=0x%08x dma_tx_phy=0x%08x\n", __func__,
(u32) priv->dma_rx_phy, (u32) priv->dma_tx_phy);
@@ -1034,8 +1076,9 @@ static void init_dma_desc_rings(struct n
else
p = priv->dma_rx + i;
- if (stmmac_init_rx_buffers(priv, p, i))
- break;
+ ret = stmmac_init_rx_buffers(priv, p, i);
+ if (ret)
+ goto err_init_rx_buffers;
if (netif_msg_probe(priv))
pr_debug("[%p]\t[%p]\t[%x]\n", priv->rx_skbuff[i],
@@ -1081,20 +1124,44 @@ static void init_dma_desc_rings(struct n
if (netif_msg_hw(priv))
stmmac_display_rings(priv);
+
+ return 0;
+err_init_rx_buffers:
+ while (--i >= 0)
+ stmmac_free_rx_buffers(priv, i);
+ kfree(priv->tx_skbuff);
+err_tx_skbuff:
+ kfree(priv->tx_skbuff_dma);
+err_tx_skbuff_dma:
+ kfree(priv->rx_skbuff);
+err_rx_skbuff:
+ kfree(priv->rx_skbuff_dma);
+err_rx_skbuff_dma:
+ if (priv->extend_desc) {
+ dma_free_coherent(priv->device, priv->dma_tx_size *
+ sizeof(struct dma_extended_desc),
+ priv->dma_etx, priv->dma_tx_phy);
+ dma_free_coherent(priv->device, priv->dma_rx_size *
+ sizeof(struct dma_extended_desc),
+ priv->dma_erx, priv->dma_rx_phy);
+ } else {
+ dma_free_coherent(priv->device,
+ priv->dma_tx_size * sizeof(struct dma_desc),
+ priv->dma_tx, priv->dma_tx_phy);
+ dma_free_coherent(priv->device,
+ priv->dma_rx_size * sizeof(struct dma_desc),
+ priv->dma_rx, priv->dma_rx_phy);
+ }
+err_dma:
+ return ret;
}
static void dma_free_rx_skbufs(struct stmmac_priv *priv)
{
int i;
- for (i = 0; i < priv->dma_rx_size; i++) {
- if (priv->rx_skbuff[i]) {
- dma_unmap_single(priv->device, priv->rx_skbuff_dma[i],
- priv->dma_buf_sz, DMA_FROM_DEVICE);
- dev_kfree_skb_any(priv->rx_skbuff[i]);
- }
- priv->rx_skbuff[i] = NULL;
- }
+ for (i = 0; i < priv->dma_rx_size; i++)
+ stmmac_free_rx_buffers(priv, i);
}
static void dma_free_tx_skbufs(struct stmmac_priv *priv)
@@ -1560,12 +1627,17 @@ static int stmmac_open(struct net_device
priv->dma_tx_size = STMMAC_ALIGN(dma_txsize);
priv->dma_rx_size = STMMAC_ALIGN(dma_rxsize);
priv->dma_buf_sz = STMMAC_ALIGN(buf_sz);
- init_dma_desc_rings(dev);
+
+ ret = init_dma_desc_rings(dev);
+ if (ret < 0) {
+ pr_err("%s: DMA descriptors initialization failed\n", __func__);
+ goto dma_desc_error;
+ }
/* DMA initialization and SW reset */
ret = stmmac_init_dma_engine(priv);
if (ret < 0) {
- pr_err("%s: DMA initialization failed\n", __func__);
+ pr_err("%s: DMA engine initialization failed\n", __func__);
goto init_error;
}
@@ -1672,6 +1744,7 @@ wolirq_error:
init_error:
free_dma_desc_resources(priv);
+dma_desc_error:
if (priv->phydev)
phy_disconnect(priv->phydev);
phy_error:
^ permalink raw reply
* Re: [PATCH v2 net-next 6/6] bonding: remove unused bond->vlan_list
From: Nikolay Aleksandrov @ 2013-08-09 11:44 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375981079-2936-7-git-send-email-vfalico@redhat.com>
On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
> RFC -> v1: No changes.
> v1 -> v2: No changes.
>
> There are currently no users of bond->vlan_list (other than the maintaining
> functions add/remove) - so remove it and every unneeded helper.
>
> In this patch we remove:
> vlan_list from struct bonding
> bond_next_vlan - we don't need it anymore
> struct vlan_entry - it was a helper struct for bond->vlan_list
> some bits from bond_vlan_rx_add/kill_vid() - which were related to
> bond->vlan_list
> (de)initialization of vlan_list from bond_setup/uninit
> bond_add_vlan - its only scope was to maintain bond->vlan_list
>
> We don't fully remove bond_del_vlan() - bond_alb_clear_vlan() still needs
> to be called when a vlan disappears. And we make bond_del_vlan() to not
> return anything cause it cannot fail already.
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
Very much
Acked-by: Nikolay Aleksandrov <nikolay@redhat.com>
^ permalink raw reply
* Re: [PATCH v2 net-next 5/6] bonding: convert bond_arp_send_all to use bond->dev->vlan_info
From: Nikolay Aleksandrov @ 2013-08-09 11:42 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375981079-2936-6-git-send-email-vfalico@redhat.com>
On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
> RFC -> v1: use the new __vlan_find_dev_next(), also release rcu_read_lock()
> only after we stop using the vlan_dev.
> v1 -> v2: no change.
>
> Instead of looping through bond->vlan_list, loop through
> bond->dev->vlan_info via __vlan_find_dev_next() under rcu_read_lock().
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 29 +++++++++++++----------------
> 1 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index e52e2d5..f536d05 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2440,11 +2440,10 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
>
> static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> {
> - int i, vlan_id;
> - __be32 *targets = bond->params.arp_targets;
> - struct vlan_entry *vlan;
> - struct net_device *vlan_dev = NULL;
> + struct net_device *vlan_dev;
> struct rtable *rt;
> + __be32 *targets = bond->params.arp_targets;
> + int i;
>
Style nitpick: maybe move them longest -> shortest.
> for (i = 0; (i < BOND_MAX_ARP_TARGETS); i++) {
> __be32 addr;
> @@ -2486,28 +2485,26 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> continue;
> }
>
> - vlan_id = 0;
> - list_for_each_entry(vlan, &bond->vlan_list, vlan_list) {
> - rcu_read_lock();
> - vlan_dev = __vlan_find_dev_deep(bond->dev,
> - htons(ETH_P_8021Q),
> - vlan->vlan_id);
> - rcu_read_unlock();
> + vlan_dev = NULL;
> +
> + rcu_read_lock();
> + while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
> if (vlan_dev == rt->dst.dev) {
> - vlan_id = vlan->vlan_id;
> pr_debug("basa: vlan match on %s %d\n",
> - vlan_dev->name, vlan_id);
> + vlan_dev->name,
> + vlan_dev_vlan_id(vlan_dev));
> break;
> }
> - }
>
> - if (vlan_id && vlan_dev) {
> + if (vlan_dev) {
> ip_rt_put(rt);
> addr = bond_confirm_addr(vlan_dev, targets[i], 0);
> bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> - addr, vlan_id);
> + addr, vlan_dev_vlan_id(vlan_dev));
> + rcu_read_unlock();
> continue;
> }
> + rcu_read_unlock();
>
I think these lines can be re-arranged to something shorter like:
rcu_read_lock();
vlan_dev = NULL;
while ((vlan_dev = __vlan_find_dev_next(bond->dev, vlan_dev)))
if (vlan_dev == rt->dst.dev) {
pr_debug("basa: vlan match on %s %d\n",
vlan_dev->name,
vlan_dev_vlan_id(vlan_dev));
break;
}
if (vlan_dev) {
ip_rt_put(rt);
addr = bond_confirm_addr(vlan_dev, targets[i], 0);
bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
addr, vlan_dev_vlan_id(vlan_dev));
} else {
if (net_ratelimit())
pr_warning("%s: no path to arp_ip_target %pI4 via
rt.dev %s\n",
bond->dev->name, &targets[i],
rt->dst.dev ? rt->dst.dev->name : "NULL");
ip_rt_put(rt);
}
rcu_read_unlock();
But either way is fine, I just think this one is more readable.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v2 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list
From: Veaceslav Falico @ 2013-08-09 11:24 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <5204CEF6.1030200@redhat.com>
On Fri, Aug 09, 2013 at 01:13:58PM +0200, Nikolay Aleksandrov wrote:
>On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
>> RFC -> v1: use the changed __vlan_find_dev_next, which now works with
>> vlan's net_device instead of vlan's id. Also, fix a subtle race
>> condition if we remove the only vlan while looping through
>> MAX_LP_BURST - we end up with using the old vlan_id, so set it
>> to 0 if we don't have vlans.
>> v1 -> v2: no change.
>>
>> In alb mode, we only need each vlan's id (that is on top of bond) to tag
>> learning packets, so get them via __vlan_find_dev_next(bond->dev, last_dev).
>>
>> We must also find *any* vlan (including last id stored in current_alb_vlan)
>> if we can't find anything >= current_alb_vlan id.
>>
>> For that, we verify if bond has any vlans at all, and if yes - find the
>> next vlan id after current_alb_vlan id. So, if vlan id is not 0, we tag the
>> skb.
>>
>> CC: Jay Vosburgh <fubar@us.ibm.com>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>> ---
>> drivers/net/bonding/bond_alb.c | 47 ++++++++++++++++++++++++++++-----------
>> drivers/net/bonding/bond_alb.h | 2 +-
>> 2 files changed, 35 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index 2684329..ced5753 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -974,8 +974,9 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
>> {
>> struct bonding *bond = bond_get_bond_by_slave(slave);
>> struct learning_pkt pkt;
>> + struct net_device *vlan_dev;
>> int size = sizeof(struct learning_pkt);
>> - int i;
>> + int i, vlan_id;
>Styling nitpick: maybe re-arrange them longest -> shortest ?
Yep, sure, thank you.
>
>>
>> memset(&pkt, 0, size);
>> memcpy(pkt.mac_dst, mac_addr, ETH_ALEN);
>> @@ -1000,22 +1001,42 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
>> skb->priority = TC_PRIO_CONTROL;
>> skb->dev = slave->dev;
>>
>> + rcu_read_lock();
>Would've saved a couple of lines if we could move the rcu_read_lock inside the
>if {} block but I know that you're keeping the bond_vlan_used() result here, but
>I think that with the new functions we're covered i.e. if the vlan has
>disappeared between the call to bond_vlan_used and the afterwards we'll get NULL
>and just drop the packet, but this can be reworked to continue with the packet
>without a vlan if one is not found that is to send it without tag :-)
Yeah, not a lot of sense to hold it, however I don't see right now how it
might save us lines - we need to hold vlan_dev if it's found, and need to
release the lock if it's NULL (before continue).
>> if (bond_vlan_used(bond)) {
>> - struct vlan_entry *vlan;
>> -
>> - vlan = bond_next_vlan(bond,
>> - bond->alb_info.current_alb_vlan);
>> -
>> - bond->alb_info.current_alb_vlan = vlan;
>> - if (!vlan) {
>> + /* first try to find the previously used vlan by
>> + * id, which might have gone away already
>> + */
>> + vlan_id = bond->alb_info.current_alb_vlan;
>> + vlan_dev = __vlan_find_dev_deep(bond->dev,
>> + htons(ETH_P_8021Q),
>> + vlan_id);
>> +
>> + /* search for the next one, if not found - for any */
>> + if (vlan_dev)
>> + vlan_dev = __vlan_find_dev_next(bond->dev,
>> + vlan_dev);
>> + if (!vlan_dev)
>> + vlan_dev = __vlan_find_dev_next(bond->dev,
>> + NULL);
>> +
>This part really looks ambiguous, I've left a comment that concerns it in my
>reply to patch 02.
Yep, seen that, it indeed is not quite clear (though working when you think
a bit about it :) ). I'll address your comment about 02/06 in several days,
I'm travelling now.
>
>> + if (vlan_dev) {
>> + vlan_id = vlan_dev_vlan_id(vlan_dev);
>> + bond->alb_info.current_alb_vlan = vlan_id;
>> + } else {
>> + bond->alb_info.current_alb_vlan = 0;
>> + rcu_read_unlock();
>> kfree_skb(skb);
>> continue;
>> }
>> + } else
>> + vlan_id = 0;
>Styling nitpick: if {} else {}
Yep, thanks.
>
>Cheers,
> Nik
^ permalink raw reply
* Re: [PATCH v2 net-next 3/6] bonding: make bond_alb use 8021q's dev->vlan_info instead of vlan_list
From: Nikolay Aleksandrov @ 2013-08-09 11:13 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1375981079-2936-4-git-send-email-vfalico@redhat.com>
On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
> RFC -> v1: use the changed __vlan_find_dev_next, which now works with
> vlan's net_device instead of vlan's id. Also, fix a subtle race
> condition if we remove the only vlan while looping through
> MAX_LP_BURST - we end up with using the old vlan_id, so set it
> to 0 if we don't have vlans.
> v1 -> v2: no change.
>
> In alb mode, we only need each vlan's id (that is on top of bond) to tag
> learning packets, so get them via __vlan_find_dev_next(bond->dev, last_dev).
>
> We must also find *any* vlan (including last id stored in current_alb_vlan)
> if we can't find anything >= current_alb_vlan id.
>
> For that, we verify if bond has any vlans at all, and if yes - find the
> next vlan id after current_alb_vlan id. So, if vlan id is not 0, we tag the
> skb.
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_alb.c | 47 ++++++++++++++++++++++++++++-----------
> drivers/net/bonding/bond_alb.h | 2 +-
> 2 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 2684329..ced5753 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -974,8 +974,9 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
> {
> struct bonding *bond = bond_get_bond_by_slave(slave);
> struct learning_pkt pkt;
> + struct net_device *vlan_dev;
> int size = sizeof(struct learning_pkt);
> - int i;
> + int i, vlan_id;
Styling nitpick: maybe re-arrange them longest -> shortest ?
>
> memset(&pkt, 0, size);
> memcpy(pkt.mac_dst, mac_addr, ETH_ALEN);
> @@ -1000,22 +1001,42 @@ static void alb_send_learning_packets(struct slave *slave, u8 mac_addr[])
> skb->priority = TC_PRIO_CONTROL;
> skb->dev = slave->dev;
>
> + rcu_read_lock();
Would've saved a couple of lines if we could move the rcu_read_lock inside the
if {} block but I know that you're keeping the bond_vlan_used() result here, but
I think that with the new functions we're covered i.e. if the vlan has
disappeared between the call to bond_vlan_used and the afterwards we'll get NULL
and just drop the packet, but this can be reworked to continue with the packet
without a vlan if one is not found that is to send it without tag :-)
> if (bond_vlan_used(bond)) {
> - struct vlan_entry *vlan;
> -
> - vlan = bond_next_vlan(bond,
> - bond->alb_info.current_alb_vlan);
> -
> - bond->alb_info.current_alb_vlan = vlan;
> - if (!vlan) {
> + /* first try to find the previously used vlan by
> + * id, which might have gone away already
> + */
> + vlan_id = bond->alb_info.current_alb_vlan;
> + vlan_dev = __vlan_find_dev_deep(bond->dev,
> + htons(ETH_P_8021Q),
> + vlan_id);
> +
> + /* search for the next one, if not found - for any */
> + if (vlan_dev)
> + vlan_dev = __vlan_find_dev_next(bond->dev,
> + vlan_dev);
> + if (!vlan_dev)
> + vlan_dev = __vlan_find_dev_next(bond->dev,
> + NULL);
> +
This part really looks ambiguous, I've left a comment that concerns it in my
reply to patch 02.
> + if (vlan_dev) {
> + vlan_id = vlan_dev_vlan_id(vlan_dev);
> + bond->alb_info.current_alb_vlan = vlan_id;
> + } else {
> + bond->alb_info.current_alb_vlan = 0;
> + rcu_read_unlock();
> kfree_skb(skb);
> continue;
> }
> + } else
> + vlan_id = 0;
Styling nitpick: if {} else {}
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
From: Veaceslav Falico @ 2013-08-09 11:11 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: netdev, Jay Vosburgh, Andy Gospodarek, Patrick McHardy,
David S. Miller
In-Reply-To: <5204CD4B.5080401@redhat.com>
On Fri, Aug 09, 2013 at 01:06:51PM +0200, Nikolay Aleksandrov wrote:
>On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
>> RFC -> v1: don't add vlan_uses_dev_rcu() and change vlan_uses_dev() instead
>> v1 -> v2: remove the ASSERT_RTNL(), cause we can now be called under rcu.
>>
>> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
>> id 0, and always returns true if 8021q is loaded. This creates several bad
>> situations - some warnings in __bond_release_one() because it thinks that
>> we still have vlans while removing, sending LB packets with vlan id 0 and,
>> possibly, other caused by vlan id 0.
>>
>> Fix it by changing vlan_uses_dev() to use rcu_dereference_rtnl() instead of
>> rtnl_dereference(), and thus it can already be used in bond_vlan_used()
>> under rcu_read_lock().
>>
>> By the time vlan_uses_dev() returns we cannot be sure if there were no
>> vlans added/removed, so it's basicly an optimization function.
>>
>> Also, use the vlan_uses_dev() in __bond_release_one() cause the rtnl lock
>> is held there.
>>
>> For this call to be visible in bonding.h, add include <linux/if_vlan.h>,
>> and also remove it from any other bonding file, cause they all include
>> bonding.h, and thus linux/if_vlan.h.
>>
>> CC: Jay Vosburgh <fubar@us.ibm.com>
>> CC: Andy Gospodarek <andy@greyhouse.net>
>> CC: Patrick McHardy <kaber@trash.net>
>> CC: "David S. Miller" <davem@davemloft.net>
>> CC: Nikolay Aleksandrov <nikolay@redhat.com>
>> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>> ---
>> drivers/net/bonding/bond_alb.c | 1 -
>> drivers/net/bonding/bond_main.c | 3 +--
>> drivers/net/bonding/bonding.h | 10 +++++++++-
>> net/8021q/vlan_core.c | 5 ++---
>> 4 files changed, 12 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index 3a5db7b..2684329 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -34,7 +34,6 @@
>> #include <linux/if_arp.h>
>> #include <linux/if_ether.h>
>> #include <linux/if_bonding.h>
>> -#include <linux/if_vlan.h>
>> #include <linux/in.h>
>> #include <net/ipx.h>
>> #include <net/arp.h>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 4264a76..9d1045d 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -69,7 +69,6 @@
>> #include <net/arp.h>
>> #include <linux/mii.h>
>> #include <linux/ethtool.h>
>> -#include <linux/if_vlan.h>
>> #include <linux/if_bonding.h>
>> #include <linux/jiffies.h>
>> #include <linux/preempt.h>
>> @@ -1953,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
>> bond_set_carrier(bond);
>> eth_hw_addr_random(bond_dev);
>>
>> - if (bond_vlan_used(bond)) {
>> + if (vlan_uses_dev(bond_dev)) {
>> pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
>> bond_dev->name, bond_dev->name);
>> pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 4bf52d5..9c4539e 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -23,6 +23,7 @@
>> #include <linux/netpoll.h>
>> #include <linux/inetdevice.h>
>> #include <linux/etherdevice.h>
>> +#include <linux/if_vlan.h>
>> #include "bond_3ad.h"
>> #include "bond_alb.h"
>>
>> @@ -267,9 +268,16 @@ struct bonding {
>> #endif /* CONFIG_DEBUG_FS */
>> };
>>
>> +/* use vlan_uses_dev() if under rtnl */
>Small nitpick - I don't think this comment is necessary as both calls now reduce
>to vlan_uses_dev() and can be used interchangeably, maybe it's better to stick
>to bond_vlan_used() just for consistency everywhere.
>Anyway, I'm okay with this version as well:
Good catch, leftover from previous version.
>
>Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
Thank you.
>
^ permalink raw reply
* Re: [PATCH v2 net-next 2/6] vlan: add __vlan_find_dev_next()
From: Nikolay Aleksandrov @ 2013-08-09 11:07 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: netdev, Patrick McHardy, David S. Miller
In-Reply-To: <1375981079-2936-3-git-send-email-vfalico@redhat.com>
On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
> RFC -> v1: make the function accept/return vlan's net_device, this way we
> won't have troubles with VLAN 0, and the user code will be
> cleaner and faster.
> v1 -> v2: don't check for the master device - if we'll want in the future
> to convert a slave device - it will fail, but now we don't
> actually care.
>
> Add a new exported function __vlan_find_dev_next(dev, vlan_dev), which
> returns the a vlan's net_device that is used by the dev and is its id is
> greater or equal to vlan_dev's vlan id. If vlan_dev is NULL, return first
> vlan, if nothing is found return NULL.
>
> This function must be under rcu_read_lock(), is aware of master devices and
> doesn't guarantee that, once it returns, the vlan dev will still be used by
> dev as its vlan.
>
> It's basically a helper for "for_each_vlan_in_dev(dev, vlan_dev)" logic,
> and is supposed to be used like this:
>
> vlan_dev = NULL;
>
> while ((vlan_dev = __vlan_find_dev_next(dev, vlan_dev))) {
> if (!vlan_dev)
> continue;
>
> do_work(vlan_dev);
> }
>
> In that case we're sure that vlan_dev at least was used as a vlan, and won't
> go away while we're holding rcu_read_lock().
>
> However, if we don't hold rtnl_lock(), we can't be sure that that vlan_dev
> is still in dev's vlan_list.
>
> CC: Patrick McHardy <kaber@trash.net>
> CC: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
We already discussed privately my comments about this new function, I'll leave
them here just for the sake of having them documented:
My proposition is to drop these changes altogether (the new function, and the
vlan.h macro change) and instead add something like the following (I haven't
tested it, it's only to illustrate the idea) in if_vlan.h so it'll be accessible
to everyone:
Version 1 (circular so you can simplify the bonding ALB code, I haven't left
spaces because of auto-wrapping):
#define dev_for_each_vlan_from(dev, vlandev, proto, from, i)
for (i=from+1, vlandev=__vlan_find_dev_deep(dev, proto, from); \
i!=from; \
i=(i+1)%VLAN_N_VID, vlandev=__vlan_find_dev_deep(dev, proto, i)) \
if (vlandev)
#define dev_for_each_vlan(dev, vlandev, proto, i)
dev_for_each_vlan_from(dev, vlandev, proto, 0, i)
Version 2 is the same but a little shorter, without the circular part.
This way you reuse the already provided function __vlan_find_dev_deep and the
churn is smaller, also __vlan_find_dev_deep takes care of the master issue (that
is if dev doesn't have a vlan_info, then its master's vlan_info will be used).
Also the code will look much nicer IMO changing this:
while ((vlan_dev = __vlan_find_dev_next))
to
dev_for_each_vlan(dev, vlan_dev, proto, i)
There're also 2 nice side-effects, first you'll only walk over 4096 entries (for
the specified vlan proto only) and the bonding ALB code will simplify from the
ambiguous looking:
vlan_id = bond->alb_info.current_alb_vlan;
vlan_dev = __vlan_find_dev_deep(bond->dev,
htons(ETH_P_8021Q),
vlan_id);
/* search for the next one, if not found - for any */
if (vlan_dev)
vlan_dev = __vlan_find_dev_next(bond->dev,
vlan_dev);
if (!vlan_dev)
vlan_dev = __vlan_find_dev_next(bond->dev,
NULL);
if (vlan_dev) {
vlan_id = vlan_dev_vlan_id(vlan_dev);
bond->alb_info.current_alb_vlan = vlan_id;
} else {
bond->alb_info.current_alb_vlan = 0;
rcu_read_unlock();
kfree_skb(skb);
continue;
}
to something like (again untested, sorry for the style - line wrapping):
vlan_id = bond->alb_info.current_alb_vlan+1;
/* since from here is current+1, if there aren't any
* vlans up to current, it'll get current again if it's
* available
*/
dev_for_each_vlan_from(bond->dev, vlan_dev, htons(ETH_P_8021Q), vlan_id, i)
break;
if (vlan_dev) {
vlan_id = vlan_dev_vlan_id(vlan_dev);
bond->alb_info.current_alb_vlan = vlan_id;
} else {
bond->alb_info.current_alb_vlan = 0;
rcu_read_unlock();
kfree_skb(skb);
continue;
}
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH v2 net-next 1/6] bonding: add rcu to vlan_uses_dev() and make bond_vlan_used() use it
From: Nikolay Aleksandrov @ 2013-08-09 11:06 UTC (permalink / raw)
To: Veaceslav Falico
Cc: netdev, Jay Vosburgh, Andy Gospodarek, Patrick McHardy,
David S. Miller
In-Reply-To: <1375981079-2936-2-git-send-email-vfalico@redhat.com>
On 08/08/2013 06:57 PM, Veaceslav Falico wrote:
> RFC -> v1: don't add vlan_uses_dev_rcu() and change vlan_uses_dev() instead
> v1 -> v2: remove the ASSERT_RTNL(), cause we can now be called under rcu.
>
> Currently, bond_vlan_used() looks for any vlan, including the pseudo-vlan
> id 0, and always returns true if 8021q is loaded. This creates several bad
> situations - some warnings in __bond_release_one() because it thinks that
> we still have vlans while removing, sending LB packets with vlan id 0 and,
> possibly, other caused by vlan id 0.
>
> Fix it by changing vlan_uses_dev() to use rcu_dereference_rtnl() instead of
> rtnl_dereference(), and thus it can already be used in bond_vlan_used()
> under rcu_read_lock().
>
> By the time vlan_uses_dev() returns we cannot be sure if there were no
> vlans added/removed, so it's basicly an optimization function.
>
> Also, use the vlan_uses_dev() in __bond_release_one() cause the rtnl lock
> is held there.
>
> For this call to be visible in bonding.h, add include <linux/if_vlan.h>,
> and also remove it from any other bonding file, cause they all include
> bonding.h, and thus linux/if_vlan.h.
>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: Patrick McHardy <kaber@trash.net>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Nikolay Aleksandrov <nikolay@redhat.com>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_alb.c | 1 -
> drivers/net/bonding/bond_main.c | 3 +--
> drivers/net/bonding/bonding.h | 10 +++++++++-
> net/8021q/vlan_core.c | 5 ++---
> 4 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index 3a5db7b..2684329 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -34,7 +34,6 @@
> #include <linux/if_arp.h>
> #include <linux/if_ether.h>
> #include <linux/if_bonding.h>
> -#include <linux/if_vlan.h>
> #include <linux/in.h>
> #include <net/ipx.h>
> #include <net/arp.h>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 4264a76..9d1045d 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -69,7 +69,6 @@
> #include <net/arp.h>
> #include <linux/mii.h>
> #include <linux/ethtool.h>
> -#include <linux/if_vlan.h>
> #include <linux/if_bonding.h>
> #include <linux/jiffies.h>
> #include <linux/preempt.h>
> @@ -1953,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
> bond_set_carrier(bond);
> eth_hw_addr_random(bond_dev);
>
> - if (bond_vlan_used(bond)) {
> + if (vlan_uses_dev(bond_dev)) {
> pr_warning("%s: Warning: clearing HW address of %s while it still has VLANs.\n",
> bond_dev->name, bond_dev->name);
> pr_warning("%s: When re-adding slaves, make sure the bond's HW address matches its VLANs'.\n",
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index 4bf52d5..9c4539e 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -23,6 +23,7 @@
> #include <linux/netpoll.h>
> #include <linux/inetdevice.h>
> #include <linux/etherdevice.h>
> +#include <linux/if_vlan.h>
> #include "bond_3ad.h"
> #include "bond_alb.h"
>
> @@ -267,9 +268,16 @@ struct bonding {
> #endif /* CONFIG_DEBUG_FS */
> };
>
> +/* use vlan_uses_dev() if under rtnl */
Small nitpick - I don't think this comment is necessary as both calls now reduce
to vlan_uses_dev() and can be used interchangeably, maybe it's better to stick
to bond_vlan_used() just for consistency everywhere.
Anyway, I'm okay with this version as well:
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
^ permalink raw reply
* Re: [PATCH net] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: Daniel Borkmann @ 2013-08-09 10:33 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
In-Reply-To: <1376042849-19732-1-git-send-email-dborkman@redhat.com>
On 08/09/2013 12:07 PM, Daniel Borkmann wrote:
> Probably this one is quite unlikely to be triggered, but it's more safe
> to hold a pointer to asoc and packet (instead of dereferencing) and access
> both though this after we have called sctp_transport_destroy_rcu() where
> the transport is being kfree()'d. Introduced by commit 8c98653f ("sctp:
> sctp_close: fix release of bindings for deferred call_rcu's").
>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Please ignore this, will send an updated v2 in a moment.
^ permalink raw reply
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame
From: Lutz Jaenicke @ 2013-08-09 10:15 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: netdev, Ben Hutchings, David S. Miller
In-Reply-To: <5204C07F.9050703@freescale.com>
On Fri, Aug 09, 2013 at 01:12:15PM +0300, Claudiu Manoil wrote:
> Thanks,
>
> First patch seems to include the complete implementation of ethtool -A,
> fixing the pause issue for half-duplex links as well, and it also
> implements pause autoneg. (nice)
> I'm going to apply an test this code and re-spin the resulting
> patch(es).
> Are you the author of this patch?
> (gianfar: implement flow control handling)
Yes.
Best regards,
Lutz
--
Dr.-Ing. Lutz Jänicke
CTO
Innominate Security Technologies AG /protecting industrial networks/
tel: +49.30.921028-200
fax: +49.30.921028-020
Rudower Chaussee 13
D-12489 Berlin, Germany
www.innominate.com
Register Court: AG Charlottenburg, HR B 81603
Management Board: Dirk Seewald
Chairman of the Supervisory Board: Christoph Leifer
^ permalink raw reply
* Re: [PATCH 2/2][net-next] gianfar: Add ethtool -A support for pause frame
From: Claudiu Manoil @ 2013-08-09 10:12 UTC (permalink / raw)
To: Lutz Jaenicke; +Cc: netdev, Ben Hutchings, David S. Miller
In-Reply-To: <1376036790-18238-1-git-send-email-ljaenicke@innominate.com>
Thanks,
First patch seems to include the complete implementation of ethtool -A,
fixing the pause issue for half-duplex links as well, and it also
implements pause autoneg. (nice)
I'm going to apply an test this code and re-spin the resulting
patch(es).
Are you the author of this patch?
(gianfar: implement flow control handling)
Claudiu
On 8/9/2013 11:26 AM, Lutz Jaenicke wrote:
> I have been working on the same issue and came up with a similar
> modification (that additionally implements LFC if supported)
> I have attached the two patches created:
> * gianfar: implement flow control handling
> which looks quite similar to Claudiu's patches
> * gianfar: add support for LFC (Lossless Flow Control)
> which is more or less just extracted from some patch I found on the internet
> linux-2.6.29.6-mpc8308e_rdb.patch
> The patches I have taken from a custom 3.4 based kernel so they may not
> apply cleanly but maybe bits of it can be used to generate fully supported
> solution.
>
>
^ permalink raw reply
* [PATCH net] net: sctp: sctp_transport_destroy{,_rcu}: fix potential pointer corruption
From: Daniel Borkmann @ 2013-08-09 10:07 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
Probably this one is quite unlikely to be triggered, but it's more safe
to hold a pointer to asoc and packet (instead of dereferencing) and access
both though this after we have called sctp_transport_destroy_rcu() where
the transport is being kfree()'d. Introduced by commit 8c98653f ("sctp:
sctp_close: fix release of bindings for deferred call_rcu's").
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/transport.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/sctp/transport.c b/net/sctp/transport.c
index bdbbc3f..7142756 100644
--- a/net/sctp/transport.c
+++ b/net/sctp/transport.c
@@ -176,6 +176,9 @@ static void sctp_transport_destroy_rcu(struct rcu_head *head)
*/
static void sctp_transport_destroy(struct sctp_transport *transport)
{
+ struct sctp_association *asoc = transport->asoc;
+ struct sctp_packet *packet = &transport->packet;
+
if (unlikely(!transport->dead)) {
WARN(1, "Attempt to destroy undead transport %p!\n", transport);
return;
@@ -183,10 +186,10 @@ static void sctp_transport_destroy(struct sctp_transport *transport)
call_rcu(&transport->rcu, sctp_transport_destroy_rcu);
- sctp_packet_free(&transport->packet);
+ sctp_packet_free(packet);
- if (transport->asoc)
- sctp_association_put(transport->asoc);
+ if (asoc != NULL)
+ sctp_association_put(asoc);
}
/* Start T3_rtx timer if it is not already running and update the heartbeat
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v3 1/2] realtek: convert to per-chip mask
From: Peter Wu @ 2013-08-09 9:58 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Francois Romieu
In-Reply-To: <1376041490.14810.5.camel@deadeye.wl.decadent.org.uk>
On Friday 09 August 2013 11:44:50 Ben Hutchings wrote:
> On Fri, 2013-08-09 at 10:51 +0200, Peter Wu wrote:
> > On Thursday 08 August 2013 20:25:41 Ben Hutchings wrote:
> > > On Thu, 2013-07-25 at 15:36 +0200, Peter Wu wrote:
> > > [...]
> > >
> > >
> > >
> > > > @@ -35,36 +32,62 @@ enum chip_type {
> > > >
> > > >
> > > > RTL8100E2,
> > > >
> > > > };
> > > >
> > > >
> > > > -enum {
> > > > - chip_type_mask = HW_REVID(1, 1, 1, 1, 1, 1, 1, 1)
> > > > +static const char * const chip_names[] = {
> > > > + [RTL8139] = "8139",
> > > > + [RTL8139_K] = "8139-K",
> > > > + [RTL8139A] = "8139A",
> > > > + [RTL8139A_G] = "8139A-G",
> > > > + [RTL8139B] = "8139B",
> > > > + [RTL8130] = "8130",
> > > > + [RTL8139C] = "8139C",
> > > > + [RTL8100] = "8100",
> > > > + [RTL8100B_8139D] = "8100B/8139D",
> > > > + [RTL8139C] = "8139C+",
> > >
> > >
> > >
> > > Shouldn't the index here be RTL8139Cp?
> >
> >
> >
> > The specifications mention "RTL8139C+", I took pre-8169 names from the
> > previous
> > rtl_info_tbl contents:
> [...]
>
> I'm talking about the index, not the name. This has an index of
> RTL8139C which is also used 3 lines up.
Oh oops, my sed expression missed this one. I'll fix this in the next version.
Regards,
Peter
^ permalink raw reply
* Re: [PATCH v3 1/2] realtek: convert to per-chip mask
From: Ben Hutchings @ 2013-08-09 9:44 UTC (permalink / raw)
To: Peter Wu; +Cc: netdev, Francois Romieu
In-Reply-To: <6171378.LP1RDUmkgD@al>
On Fri, 2013-08-09 at 10:51 +0200, Peter Wu wrote:
> On Thursday 08 August 2013 20:25:41 Ben Hutchings wrote:
> > On Thu, 2013-07-25 at 15:36 +0200, Peter Wu wrote:
> > [...]
> >
> > > @@ -35,36 +32,62 @@ enum chip_type {
> > >
> > > RTL8100E2,
> > >
> > > };
> > >
> > > -enum {
> > > - chip_type_mask = HW_REVID(1, 1, 1, 1, 1, 1, 1, 1)
> > > +static const char * const chip_names[] = {
> > > + [RTL8139] = "8139",
> > > + [RTL8139_K] = "8139-K",
> > > + [RTL8139A] = "8139A",
> > > + [RTL8139A_G] = "8139A-G",
> > > + [RTL8139B] = "8139B",
> > > + [RTL8130] = "8130",
> > > + [RTL8139C] = "8139C",
> > > + [RTL8100] = "8100",
> > > + [RTL8100B_8139D] = "8100B/8139D",
> > > + [RTL8139C] = "8139C+",
> >
> > Shouldn't the index here be RTL8139Cp?
>
> The specifications mention "RTL8139C+", I took pre-8169 names from the previous
> rtl_info_tbl contents:
[...]
I'm talking about the index, not the name. This has an index of
RTL8139C which is also used 3 lines up.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* [PATCH] can: pcan_usb: fix wrong memcpy() bytes length
From: Marc Kleine-Budde @ 2013-08-09 9:44 UTC (permalink / raw)
To: netdev
Cc: kernel, linux-can, davem, Stephane Grosjean, linux-stable,
Marc Kleine-Budde
In-Reply-To: <1376041446-24058-1-git-send-email-mkl@pengutronix.de>
From: Stephane Grosjean <s.grosjean@peak-system.com>
Fix possibly wrong memcpy() bytes length since some CAN records received from
PCAN-USB could define a DLC field in range [9..15].
In that case, the real DLC value MUST be used to move forward the record pointer
but, only 8 bytes max. MUST be copied into the data field of the struct
can_frame object of the skb given to the network core.
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/usb/peak_usb/pcan_usb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/usb/peak_usb/pcan_usb.c b/drivers/net/can/usb/peak_usb/pcan_usb.c
index 25723d8..925ab8e 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb.c
@@ -649,7 +649,7 @@ static int pcan_usb_decode_data(struct pcan_usb_msg_context *mc, u8 status_len)
if ((mc->ptr + rec_len) > mc->end)
goto decode_failed;
- memcpy(cf->data, mc->ptr, rec_len);
+ memcpy(cf->data, mc->ptr, cf->can_dlc);
mc->ptr += rec_len;
}
--
1.8.3.1
^ permalink raw reply related
* pull-request: can 2013-08-09
From: Marc Kleine-Budde @ 2013-08-09 9:44 UTC (permalink / raw)
To: netdev; +Cc: kernel, linux-can, davem
here is a fix for the v3.11 release cycle:
Stephane Grosjean has found and fixed a potential wrong length for the mempy()
in pcan_usb's decode usb data function.
regards,
Marc
^ 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