* Re: [PATCH] solos-pci: don't call vcc->pop() after pclose()
From: chas williams - CONTRACTOR @ 2012-11-29 14:41 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: David Woodhouse, David Laight, davem, netdev, linux-kernel,
nathan
In-Reply-To: <20121129124344.GA7704@shrek.podlesie.net>
On Thu, 29 Nov 2012 13:43:44 +0100
Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> Removing packets from tx_queue is not needed. We can transmit packets
> also after close. We just can't call vcc->pop() after close,
> so we can just set SKB_CB(skb)->vcc of such packets to NULL so fpga_tx()
> won't call vcc->pop().
i dont think you can transmit packets after close(). you can transmit
packets during close() though. if you transmit after close that means
that you are using the vpi/vci pair that the atm stack thinks is no
longer in use. additionally after close(), the hardware should be in a
state such that you cannot transmit or receive on the vpi/vci that has
been closed.
close() needs to make sure that any pending tx packets are sent or
otherwise disposed of (like turning off the transmit segmentation
engine, clearing the packets, or whatever). any partially reassembled
pdu's also need to be cleared as well.
^ permalink raw reply
* Re: [net-next RFC v2] net_cls: traffic counter based on classification control cgroup
From: Daniel Wagner @ 2012-11-29 14:36 UTC (permalink / raw)
To: Alexey Perevalov
Cc: Glauber Costa, netdev-u79uwXL29TY76Z2rM5mHXA,
cgroups-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <50B5F2EE.6050204-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hi Alexey
On 28.11.2012 12:18, Alexey Perevalov wrote:
> On 11/28/2012 12:09 PM, Daniel Wagner wrote:
>>>>> 2) When Daniel exposed his use case to me, it gave me the impression
>>>>> that "counting traffic" is something that is totally doable by
>>>>> having a
>>>>> dedicated interface in a separate namespace. Basically, we already
>>>>> count
>>>>> traffic (rx and tx) for all interfaces anyway, so it suggests that it
>>>>> could be an interesting way to see the problem.
>>>> Moving applications into separate net namespaces is for sure a valid
>>>> solution.
>>>> Though there is a one drawback in this approach. The namespaces need
>>>> to be
>>>> attached to a bridge and then some NATting. That means every
>>>> application
>>>> would get it's own IP address. This might be okay for your certain use
>>>> cases but I am still trying to work around this. Glauber and I had some
>>>> discussion about this and he suggested to allow the physical networking
>>>> device to be attached to several namespaces (e.g. via macvlan). Every
>>>> namespace would get the same IP address. Unfortunately, this would
>>>> result in
>>>> the same mess as several physical devices on a network get the same
>>>> IP address assigned.
>>> Is I truly understand what to make statistics works we need to put
>>> process to separate namespace?
>> If a process lives in its own network namespace then you can
>> count the packets/bytes on the network interface level. The side effect
>> is that is that each namespace is obviously a new network and has to be
>> treated as such.
>>
>>> Approach to keep counter in cgroup hasn't such side effects, but it has
>>> another ).
>> cgroups are not for free. Currently a lot of effort is put into getting
>> a reasonable performance and behavior into cgroups. In this situation
>> any new feature added to cgroups will need a pretty good justification
>> why it is needed and why it cant be done with existing infrastructure.
> I want to figure out in yours proposed design:
>
> +------------------------------------------------+
> |network namespace1: pid1, pid2,... |
> | |
> +---------------------------+
> | network stack, network iface |
> | |
> | nf hooks +------->| physical
> network |
> +------------------------------------------------+ |
> interface |
> | |
> | |
> +------------------------------------------------+
> | |
> |network namespace2: pid1, pid2,... |
> | |
> | +------->| |
> | network stack, network iface |
> | |
> | nf hooks |
> | |
> +------------------------------------------------+
> | |
> +---------------------------+
> ... ^
> +------------------------------------------------+ |
> |network namespace3: pid1, pid2,... | |
> | | |
> | network stack, network iface +-----------+
> | nf hooks |
> +------------------------------------------------+
>
>
> Question, in case of one physical networking device connected to several
> namespaces,
Currently, a physical device can only live in one namespace. The idea
was to see if that 'limitation' could be removed, e.g. by modifying macvlan.
> is it allow to tweak network packet scheduler (qdisc instance) using
> traffic control tool for one physical network interface?
> The same question is about netfilter hooks. I have seen the code, it
> seems to me nf hooks is registering per network stack now.
As I said this was just and idea and maybe it is not possible.
> CGroup framework has an notification mechanism based on eventfd. For
> example I can just send notification to user space about network activity.
> Is there such mechanism in standard infrastructure to notify user space
> apps on activity on monitored application (maybe nf_queue)?
My current plan is to use IDLETIMER for this stuff.
cheers,
daniel
^ permalink raw reply
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: chas williams - CONTRACTOR @ 2012-11-29 14:29 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: David Woodhouse, David Laight, davem, netdev, linux-kernel,
nathan
In-Reply-To: <20121129105715.GA10226@shrek.podlesie.net>
On Thu, 29 Nov 2012 11:57:15 +0100
Krzysztof Mazur <krzysiek@podlesie.net> wrote:
> or if we really want to call vcc->pop() for such skbs:
you need to call ->pop() to cleaning up the wmem_alloc accounting.
otherwise you will get complaints from the atm stack later about
freeing a vcc that had outstanding data.
^ permalink raw reply
* iputils: ping -I <iface>
From: Jan Synacek @ 2012-11-29 14:12 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev
Hello,
There seems to be a bug(?) when calling ping with -I lo:
$ ping -I lo kernel.org
PING kernel.org (149.20.4.69) from 192.168.1.10 lo: 56(84) bytes of data.
^C
Note that 192.168.1.10 is my primary interface's address (em1). However, no
replies are coming back.
$ ping -I em1 kernel.org
PING kernel.org (149.20.4.69) from 192.168.1.10 em1: 56(84) bytes of data.
64 bytes from pub2.kernel.org (149.20.4.69): icmp_seq=1 ttl=42 time=202 ms
64 bytes from pub2.kernel.org (149.20.4.69): icmp_seq=2 ttl=42 time=187 ms
^C
Works as expected.
I know that binding to loopback probably doesn't make much sense, but I think
that ping should be able to cope with that.
Also, it would be nice to mention the difference between -I <ip> and -I <iface>
in the manpage.
I don't understand the problem clearly enough to write a patch.
Regards,
--
Jan Synacek
Software Engineer, BaseOS team Brno, Red Hat
^ permalink raw reply
* pull-request: can-next 2012-11-29
From: Marc Kleine-Budde @ 2012-11-29 13:50 UTC (permalink / raw)
To: netdev; +Cc: linux-can@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2152 bytes --]
Hello David,
this pull request is for net-next/master. There is a patch by Alexander
Stein fixing a reference counter problem which can make driver
unloading impossible (stable Cc'ed). And several patches by me which
remove an obsolete mechanism from several drivers, which is already
handled at the infrastructure level.
regards,
Marc
---
The following changes since commit 3177bf6f922f62743133abbcbbbb5545f4133b2d:
net: ethernet: cpsw: fix build warnings for CPSW when CPTS not selected (2012-11-28 17:51:16 -0500)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can-next.git for-davem
for you to fetch changes up to 823d7a1f761d6404babaab04cc8b1724186cf2c8:
can: pcan_usb_core: remove obsolete variable open_time (2012-11-29 14:34:06 +0100)
----------------------------------------------------------------
Alexander Stein (1):
can: Do not call dev_put if restart timer is running upon close
Marc Kleine-Budde (5):
can: mscan: remove obsolete variable open_time
can: sja1000: remove obsolete variable open_time
can: ems_usb: remove obsolete variable open_time
can: esd_usb2: remove obsolete variable open_time
can: pcan_usb_core: remove obsolete variable open_time
drivers/net/can/dev.c | 3 +--
drivers/net/can/mscan/mscan.c | 8 --------
drivers/net/can/mscan/mscan.h | 1 -
drivers/net/can/sja1000/sja1000.c | 8 --------
drivers/net/can/sja1000/sja1000.h | 1 -
drivers/net/can/usb/ems_usb.c | 7 -------
drivers/net/can/usb/esd_usb2.c | 10 ----------
drivers/net/can/usb/peak_usb/pcan_usb_core.c | 5 -----
drivers/net/can/usb/peak_usb/pcan_usb_core.h | 1 -
9 files changed, 1 insertion(+), 43 deletions(-)
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 261 bytes --]
^ permalink raw reply
* Re: [PATCH] solos-pci: don't call vcc->pop() after pclose()
From: Krzysztof Mazur @ 2012-11-29 13:20 UTC (permalink / raw)
To: David Woodhouse
Cc: David Laight, chas williams - CONTRACTOR, davem, netdev,
linux-kernel, nathan
In-Reply-To: <1354193837.21562.159.camel@shinybook.infradead.org>
On Thu, Nov 29, 2012 at 12:57:17PM +0000, David Woodhouse wrote:
> On Thu, 2012-11-29 at 13:43 +0100, Krzysztof Mazur wrote:
> >
> > Removing packets from tx_queue is not needed. We can transmit packets
> > also after close. We just can't call vcc->pop() after close,
> > so we can just set SKB_CB(skb)->vcc of such packets to NULL so
> > fpga_tx() won't call vcc->pop().
>
> Your patch doesn't do that, does it? You'd want something like
>
> if (card->tx_skb[port] && SKB_CB(card->tx_skb[port]->vcc) == vcc)
> SKB_CB(card->tx_skb[port]->vcc) = NULL;
No, I want to process all queued packets, not just only those that
were already sent do card.
In that case we will need to remove other packets with that vcc
from queue, of couse we can still do that in the same loop, something
like:
if (SKB_CB(skb)->vcc == vcc) {
if (card->tx_skb[port] == skb) {
skb_get(skb);
solos_pop(SKB_CB(skb)->vcc, skb);
SKB_CB(skb)->vcc = NULL;
} else {
skb_unlink(skb, &card->tx_queue[port]);
solos_pop(SKB_CB(skb)->vcc, skb);
}
}
But I don't think that this optization is needed.
>
> Under card->tx_lock should suffice.
>
> And do we just *not* call the ->pop() on that skb ever? And hope that it
> doesn't screw up some other state somewhere? Like if we're doing MLPPP
> and I've implemented BQL for PPP... we might never call
> ppp_completed_queue() for that skb, so even though this *channel* is
> going away, it might still contribute towards the perceived queue on the
> overall PPP netdev?
>
> Failing to call ->pop() could cause memory leaks and other issues; I
> don't think it's reasonable. I think we *have* to wait for
> card->tx_skb[port] if it's for the VCC we're closing.
We are calling ->pop() in solos_pop() just before SKB_CB(skb)->vcc = NULL,
but we are doing that before we really finish processing that packet,
that's why we do skb_get(skb).
Krzysiek
^ permalink raw reply
* Re: [PATCH] solos-pci: don't call vcc->pop() after pclose()
From: David Woodhouse @ 2012-11-29 12:57 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: David Laight, chas williams - CONTRACTOR, davem, netdev,
linux-kernel, nathan
In-Reply-To: <20121129124344.GA7704@shrek.podlesie.net>
[-- Attachment #1: Type: text/plain, Size: 1095 bytes --]
On Thu, 2012-11-29 at 13:43 +0100, Krzysztof Mazur wrote:
>
> Removing packets from tx_queue is not needed. We can transmit packets
> also after close. We just can't call vcc->pop() after close,
> so we can just set SKB_CB(skb)->vcc of such packets to NULL so
> fpga_tx() won't call vcc->pop().
Your patch doesn't do that, does it? You'd want something like
if (card->tx_skb[port] && SKB_CB(card->tx_skb[port]->vcc) == vcc)
SKB_CB(card->tx_skb[port]->vcc) = NULL;
Under card->tx_lock should suffice.
And do we just *not* call the ->pop() on that skb ever? And hope that it
doesn't screw up some other state somewhere? Like if we're doing MLPPP
and I've implemented BQL for PPP... we might never call
ppp_completed_queue() for that skb, so even though this *channel* is
going away, it might still contribute towards the perceived queue on the
overall PPP netdev?
Failing to call ->pop() could cause memory leaks and other issues; I
don't think it's reasonable. I think we *have* to wait for
card->tx_skb[port] if it's for the VCC we're closing.
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* [PATCH] solos-pci: don't call vcc->pop() after pclose()
From: Krzysztof Mazur @ 2012-11-29 12:43 UTC (permalink / raw)
To: David Woodhouse
Cc: David Laight, chas williams - CONTRACTOR, davem, netdev,
linux-kernel, nathan
In-Reply-To: <1354190143.21562.145.camel@shinybook.infradead.org>
On Thu, Nov 29, 2012 at 11:55:43AM +0000, David Woodhouse wrote:
> On Thu, 2012-11-29 at 11:57 +0100, Krzysztof Mazur wrote:
> > do we really need to wait here?
> > Why don't just do something like that:
> >
> > tasklet_disable(&card->tlet);
> > spin_lock(&card->tx_queue_lock);
> > for each skb in queue
> > SKB_CB(skb)->vcc = NULL;
> > spin_unlock(&card->tx_queue_lock);
> > tasklet_enable(&card->tlet);
> >
> > or if we really want to call vcc->pop() for such skbs:
> >
> > tasklet_disable(&card->tlet);
> > spin_lock(&card->tx_queue_lock);
> > for each skb in queue {
> > skb_get(skb);
> > solos_pop(SKB_CB(skb)->vcc, skb);
> > SKB_CB(skb)->vcc = NULL;
> > }
> > spin_unlock(&card->tx_queue_lock);
> > tasklet_enable(&card->tlet);
>
> Yes, we could certainly remove the packets from the tx_queue first.
>
> However, in the card->using_dma case there might be a skb for this vcc
> *currently* being DMA'd, and we'd still need to wait for that one.
Removing packets from tx_queue is not needed. We can transmit packets
also after close. We just can't call vcc->pop() after close,
so we can just set SKB_CB(skb)->vcc of such packets to NULL so fpga_tx()
won't call vcc->pop().
Maybe I was not precise enough, I'm think that all we need is
something like that:
-- >8 --
Subject: [PATCH] solos-pci: don't call vcc->pop() after pclose()
After atmdev_ops->close() we cannot use vcc->pop() because the vcc may,
and probably will be destroyed.
We can just set vcc for such frames to NULL because fpga_tx() after
completion will call dev_kfree_skb() in that case.
Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>
---
drivers/atm/solos-pci.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
index 9851093..aabe021 100644
--- a/drivers/atm/solos-pci.c
+++ b/drivers/atm/solos-pci.c
@@ -868,6 +868,19 @@ static void pclose(struct atm_vcc *vcc)
struct solos_card *card = vcc->dev->dev_data;
struct sk_buff *skb;
struct pkt_hdr *header;
+ unsigned int port;
+
+ tasklet_disable(&card->tlet);
+ spin_lock(&card->tx_queue_lock);
+ for (port = 0; port < card->nr_ports; port++)
+ skb_queue_walk(&card->tx_queue[port], skb)
+ if (SKB_CB(skb)->vcc == vcc) {
+ skb_get(skb);
+ solos_pop(SKB_CB(skb)->vcc, skb);
+ SKB_CB(skb)->vcc = NULL;
+ }
+ spin_unlock(&card->tx_queue_lock);
+ tasklet_enable(&card->tlet);
skb = alloc_skb(sizeof(*header), GFP_ATOMIC);
if (!skb) {
--
1.8.0.411.g71a7da8
^ permalink raw reply related
* [PATCH v2] bonding: fix race condition in bonding_store_slaves_active
From: Nikolay Aleksandrov @ 2012-11-29 11:37 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <1353759595-30452-1-git-send-email-nikolay@redhat.com>
Race between bonding_store_slaves_active() and slave manipulation
functions. The bond_for_each_slave use in bonding_store_slaves_active()
is not protected by any synchronization mechanism.
NULL pointer dereference is easy to reach.
Fixed by acquiring the bond->lock for the slave walk.
v2: Make description text < 75 columns
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_sysfs.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ef8d2a0..ba4f95b 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1582,6 +1582,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
goto out;
}
+ read_lock(&bond->lock);
bond_for_each_slave(bond, slave, i) {
if (!bond_is_active_slave(slave)) {
if (new_value)
@@ -1590,6 +1591,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
slave->inactive = 1;
}
}
+ read_unlock(&bond->lock);
out:
return ret;
}
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: David Woodhouse @ 2012-11-29 11:55 UTC (permalink / raw)
To: Krzysztof Mazur
Cc: David Laight, chas williams - CONTRACTOR, davem, netdev,
linux-kernel, nathan
In-Reply-To: <20121129105715.GA10226@shrek.podlesie.net>
[-- Attachment #1: Type: text/plain, Size: 1675 bytes --]
On Thu, 2012-11-29 at 11:57 +0100, Krzysztof Mazur wrote:
> do we really need to wait here?
> Why don't just do something like that:
>
> tasklet_disable(&card->tlet);
> spin_lock(&card->tx_queue_lock);
> for each skb in queue
> SKB_CB(skb)->vcc = NULL;
> spin_unlock(&card->tx_queue_lock);
> tasklet_enable(&card->tlet);
>
> or if we really want to call vcc->pop() for such skbs:
>
> tasklet_disable(&card->tlet);
> spin_lock(&card->tx_queue_lock);
> for each skb in queue {
> skb_get(skb);
> solos_pop(SKB_CB(skb)->vcc, skb);
> SKB_CB(skb)->vcc = NULL;
> }
> spin_unlock(&card->tx_queue_lock);
> tasklet_enable(&card->tlet);
Yes, we could certainly remove the packets from the tx_queue first.
However, in the card->using_dma case there might be a skb for this vcc
*currently* being DMA'd, and we'd still need to wait for that one.
I suppose we could just have a waitqueue in *every* TX skb, and under
card->tx_lock we could add ourselves to *that* waitqueue. Or just a
global waitqueue for DMA tx_done, perhaps. But waiting for our own
PKT_PCLOSE skb is just 'cleaner' in my view. It's simpler, and it's much
easier to test. Even if I had DMA-capable hardware, I'd have to get the
right timing to properly test that TX-pending-DMA case.
So dequeuing the packets would only serve to make pclose() slightly
faster, rather than simplifying it. It's hardly a fast path that we care
about, and I've also already ensured that there should only be one or
two packets queued per vcc *anyway*. So I'm mostly inclined not to
bother.
(I did fix the timeout argument to wait_for_completion_timeout())
--
dwmw2
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6171 bytes --]
^ permalink raw reply
* [PATCH v3] bonding: make arp_ip_target parameter checks consistent with sysfs
From: Nikolay Aleksandrov @ 2012-11-29 11:34 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <1354121052-6263-1-git-send-email-nikolay@redhat.com>
The module can be loaded with arp_ip_target="255.255.255.255" which makes
it impossible to remove as the function in sysfs checks for that value,
so we make the parameter checks consistent with sysfs.
v2: Fix formatting
v3: Make description text < 75 columns
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5f5b69f..d29159a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4706,12 +4706,13 @@ static int bond_check_params(struct bond_params *params)
arp_ip_count++) {
/* not complete check, but should be good enough to
catch mistakes */
- if (!isdigit(arp_ip_target[arp_ip_count][0])) {
+ __be32 ip = in_aton(arp_ip_target[arp_ip_count]);
+ if (!isdigit(arp_ip_target[arp_ip_count][0]) ||
+ ip == 0 || ip == htonl(INADDR_BROADCAST)) {
pr_warning("Warning: bad arp_ip_target module parameter (%s), ARP monitoring will not be performed\n",
arp_ip_target[arp_ip_count]);
arp_interval = 0;
} else {
- __be32 ip = in_aton(arp_ip_target[arp_ip_count]);
arp_target[arp_ip_count] = ip;
}
}
--
1.7.11.7
^ permalink raw reply related
* [PATCH v2] bonding: fix miimon and arp_interval delayed work race conditions
From: Nikolay Aleksandrov @ 2012-11-29 11:31 UTC (permalink / raw)
To: netdev; +Cc: fubar, andy, davem
In-Reply-To: <1353759471-30323-1-git-send-email-nikolay@redhat.com>
First I would give three observations which will be used later.
Observation 1: if (delayed_work_pending(wq)) cancel_delayed_work(wq)
This usage is wrong because the pending bit is cleared just before the
work's fn is executed and if the function re-arms itself we might end up
with the work still running. It's safe to call cancel_delayed_work_sync()
even if the work is not queued at all.
Observation 2: Use of INIT_DELAYED_WORK()
Work needs to be initialized only once prior to (de/en)queueing.
Observation 3: IFF_UP is set only after ndo_open is called
Related race conditions:
1. Race between bonding_store_miimon() and bonding_store_arp_interval()
Because of Obs.1 we can end up having both works enqueued.
2. Multiple races with INIT_DELAYED_WORK()
Since the works are not protected by anything between INIT_DELAYED_WORK()
and calls to (en/de)queue it is possible for races between the following
functions:
(races are also possible between the calls to INIT_DELAYED_WORK()
and workqueue code)
bonding_store_miimon() - bonding_store_arp_interval(), bond_close(),
bond_open(), enqueued functions
bonding_store_arp_interval() - bonding_store_miimon(), bond_close(),
bond_open(), enqueued functions
3. By Obs.1 we need to change bond_cancel_all()
Bugs 1 and 2 are fixed by moving all work initializations in bond_open
which by Obs. 2 and Obs. 3 and the fact that we make sure that all works
are cancelled in bond_close(), is guaranteed not to have any work
enqueued.
Also RTNL lock is now acquired in bonding_store_miimon/arp_interval so
they can't race with bond_close and bond_open. The opposing work is
cancelled only if the IFF_UP flag is set and it is cancelled
unconditionally. The opposing work is already cancelled if the interface
is down so no need to cancel it again. This way we don't need new
synchronizations for the bonding workqueue. These bugs (and fixes) are
tied together and belong in the same patch.
Note: I have left 1 line intentionally over 80 characters (84) because I
didn't like how it looks broken down. If you'd prefer it otherwise,
then simply break it.
v2: Make description text < 75 columns
Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
drivers/net/bonding/bond_main.c | 88 ++++++++++++----------------------------
drivers/net/bonding/bond_sysfs.c | 34 +++++-----------
2 files changed, 36 insertions(+), 86 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5f5b69f..1445c7d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3459,6 +3459,28 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
/*-------------------------- Device entry points ----------------------------*/
+static void bond_work_init_all(struct bonding *bond)
+{
+ INIT_DELAYED_WORK(&bond->mcast_work,
+ bond_resend_igmp_join_requests_delayed);
+ INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
+ INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor);
+ if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
+ INIT_DELAYED_WORK(&bond->arp_work, bond_activebackup_arp_mon);
+ else
+ INIT_DELAYED_WORK(&bond->arp_work, bond_loadbalance_arp_mon);
+ INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
+}
+
+static void bond_work_cancel_all(struct bonding *bond)
+{
+ cancel_delayed_work_sync(&bond->mii_work);
+ cancel_delayed_work_sync(&bond->arp_work);
+ cancel_delayed_work_sync(&bond->alb_work);
+ cancel_delayed_work_sync(&bond->ad_work);
+ cancel_delayed_work_sync(&bond->mcast_work);
+}
+
static int bond_open(struct net_device *bond_dev)
{
struct bonding *bond = netdev_priv(bond_dev);
@@ -3481,41 +3503,27 @@ static int bond_open(struct net_device *bond_dev)
}
read_unlock(&bond->lock);
- INIT_DELAYED_WORK(&bond->mcast_work, bond_resend_igmp_join_requests_delayed);
+ bond_work_init_all(bond);
if (bond_is_lb(bond)) {
/* bond_alb_initialize must be called before the timer
* is started.
*/
- if (bond_alb_initialize(bond, (bond->params.mode == BOND_MODE_ALB))) {
- /* something went wrong - fail the open operation */
+ if (bond_alb_initialize(bond, (bond->params.mode == BOND_MODE_ALB)))
return -ENOMEM;
- }
-
- INIT_DELAYED_WORK(&bond->alb_work, bond_alb_monitor);
queue_delayed_work(bond->wq, &bond->alb_work, 0);
}
- if (bond->params.miimon) { /* link check interval, in milliseconds. */
- INIT_DELAYED_WORK(&bond->mii_work, bond_mii_monitor);
+ if (bond->params.miimon) /* link check interval, in milliseconds. */
queue_delayed_work(bond->wq, &bond->mii_work, 0);
- }
if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
- if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
- INIT_DELAYED_WORK(&bond->arp_work,
- bond_activebackup_arp_mon);
- else
- INIT_DELAYED_WORK(&bond->arp_work,
- bond_loadbalance_arp_mon);
-
queue_delayed_work(bond->wq, &bond->arp_work, 0);
if (bond->params.arp_validate)
bond->recv_probe = bond_arp_rcv;
}
if (bond->params.mode == BOND_MODE_8023AD) {
- INIT_DELAYED_WORK(&bond->ad_work, bond_3ad_state_machine_handler);
queue_delayed_work(bond->wq, &bond->ad_work, 0);
/* register to receive LACPDUs */
bond->recv_probe = bond_3ad_lacpdu_recv;
@@ -3530,34 +3538,10 @@ static int bond_close(struct net_device *bond_dev)
struct bonding *bond = netdev_priv(bond_dev);
write_lock_bh(&bond->lock);
-
bond->send_peer_notif = 0;
-
write_unlock_bh(&bond->lock);
- if (bond->params.miimon) { /* link check interval, in milliseconds. */
- cancel_delayed_work_sync(&bond->mii_work);
- }
-
- if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
- cancel_delayed_work_sync(&bond->arp_work);
- }
-
- switch (bond->params.mode) {
- case BOND_MODE_8023AD:
- cancel_delayed_work_sync(&bond->ad_work);
- break;
- case BOND_MODE_TLB:
- case BOND_MODE_ALB:
- cancel_delayed_work_sync(&bond->alb_work);
- break;
- default:
- break;
- }
-
- if (delayed_work_pending(&bond->mcast_work))
- cancel_delayed_work_sync(&bond->mcast_work);
-
+ bond_work_cancel_all(bond);
if (bond_is_lb(bond)) {
/* Must be called only after all
* slaves have been released
@@ -4436,26 +4420,6 @@ static void bond_setup(struct net_device *bond_dev)
bond_dev->features |= bond_dev->hw_features;
}
-static void bond_work_cancel_all(struct bonding *bond)
-{
- if (bond->params.miimon && delayed_work_pending(&bond->mii_work))
- cancel_delayed_work_sync(&bond->mii_work);
-
- if (bond->params.arp_interval && delayed_work_pending(&bond->arp_work))
- cancel_delayed_work_sync(&bond->arp_work);
-
- if (bond->params.mode == BOND_MODE_ALB &&
- delayed_work_pending(&bond->alb_work))
- cancel_delayed_work_sync(&bond->alb_work);
-
- if (bond->params.mode == BOND_MODE_8023AD &&
- delayed_work_pending(&bond->ad_work))
- cancel_delayed_work_sync(&bond->ad_work);
-
- if (delayed_work_pending(&bond->mcast_work))
- cancel_delayed_work_sync(&bond->mcast_work);
-}
-
/*
* Destroy a bonding device.
* Must be under rtnl_lock when this function is called.
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ef8d2a0..3327a07 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -513,6 +513,8 @@ static ssize_t bonding_store_arp_interval(struct device *d,
int new_value, ret = count;
struct bonding *bond = to_bond(d);
+ if (!rtnl_trylock())
+ return restart_syscall();
if (sscanf(buf, "%d", &new_value) != 1) {
pr_err("%s: no arp_interval value specified.\n",
bond->dev->name);
@@ -539,10 +541,6 @@ static ssize_t bonding_store_arp_interval(struct device *d,
pr_info("%s: ARP monitoring cannot be used with MII monitoring. %s Disabling MII monitoring.\n",
bond->dev->name, bond->dev->name);
bond->params.miimon = 0;
- if (delayed_work_pending(&bond->mii_work)) {
- cancel_delayed_work(&bond->mii_work);
- flush_workqueue(bond->wq);
- }
}
if (!bond->params.arp_targets[0]) {
pr_info("%s: ARP monitoring has been set up, but no ARP targets have been specified.\n",
@@ -554,19 +552,12 @@ static ssize_t bonding_store_arp_interval(struct device *d,
* timer will get fired off when the open function
* is called.
*/
- if (!delayed_work_pending(&bond->arp_work)) {
- if (bond->params.mode == BOND_MODE_ACTIVEBACKUP)
- INIT_DELAYED_WORK(&bond->arp_work,
- bond_activebackup_arp_mon);
- else
- INIT_DELAYED_WORK(&bond->arp_work,
- bond_loadbalance_arp_mon);
-
- queue_delayed_work(bond->wq, &bond->arp_work, 0);
- }
+ cancel_delayed_work_sync(&bond->mii_work);
+ queue_delayed_work(bond->wq, &bond->arp_work, 0);
}
out:
+ rtnl_unlock();
return ret;
}
static DEVICE_ATTR(arp_interval, S_IRUGO | S_IWUSR,
@@ -962,6 +953,8 @@ static ssize_t bonding_store_miimon(struct device *d,
int new_value, ret = count;
struct bonding *bond = to_bond(d);
+ if (!rtnl_trylock())
+ return restart_syscall();
if (sscanf(buf, "%d", &new_value) != 1) {
pr_err("%s: no miimon value specified.\n",
bond->dev->name);
@@ -993,10 +986,6 @@ static ssize_t bonding_store_miimon(struct device *d,
bond->params.arp_validate =
BOND_ARP_VALIDATE_NONE;
}
- if (delayed_work_pending(&bond->arp_work)) {
- cancel_delayed_work(&bond->arp_work);
- flush_workqueue(bond->wq);
- }
}
if (bond->dev->flags & IFF_UP) {
@@ -1005,15 +994,12 @@ static ssize_t bonding_store_miimon(struct device *d,
* timer will get fired off when the open function
* is called.
*/
- if (!delayed_work_pending(&bond->mii_work)) {
- INIT_DELAYED_WORK(&bond->mii_work,
- bond_mii_monitor);
- queue_delayed_work(bond->wq,
- &bond->mii_work, 0);
- }
+ cancel_delayed_work_sync(&bond->arp_work);
+ queue_delayed_work(bond->wq, &bond->mii_work, 0);
}
}
out:
+ rtnl_unlock();
return ret;
}
static DEVICE_ATTR(miimon, S_IRUGO | S_IWUSR,
--
1.7.11.7
^ permalink raw reply related
* Re: [net-next RFC] pktgen: don't wait for the device who doesn't free skb immediately after sent
From: Michael S. Tsirkin @ 2012-11-29 11:30 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, virtualization, Stephen Hemminger, davem
In-Reply-To: <1570859.IePPxHAEX1@jason-thinkpad-t430s>
On Thu, Nov 29, 2012 at 06:13:13PM +0800, Jason Wang wrote:
> On Wednesday, November 28, 2012 08:53:05 AM Stephen Hemminger wrote:
> > On Wed, 28 Nov 2012 14:48:52 +0800
> >
> > Jason Wang <jasowang@redhat.com> wrote:
> > > On 11/28/2012 12:49 AM, Stephen Hemminger wrote:
> > > > On Tue, 27 Nov 2012 14:45:13 +0800
> > > >
> > > > Jason Wang <jasowang@redhat.com> wrote:
> > > >> On 11/27/2012 01:37 AM, Stephen Hemminger wrote:
> > > >>> On Mon, 26 Nov 2012 15:56:52 +0800
> > > >>>
> > > >>> Jason Wang <jasowang@redhat.com> wrote:
> > > >>>> Some deivces do not free the old tx skbs immediately after it has
> > > >>>> been sent
> > > >>>> (usually in tx interrupt). One such example is virtio-net which
> > > >>>> optimizes for virt and only free the possible old tx skbs during the
> > > >>>> next packet sending. This would lead the pktgen to wait forever in
> > > >>>> the refcount of the skb if no other pakcet will be sent afterwards.
> > > >>>>
> > > >>>> Solving this issue by introducing a new flag IFF_TX_SKB_FREE_DELAY
> > > >>>> which could notify the pktgen that the device does not free skb
> > > >>>> immediately after it has been sent and let it not to wait for the
> > > >>>> refcount to be one.
> > > >>>>
> > > >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> > > >>>
> > > >>> Another alternative would be using skb_orphan() and skb->destructor.
> > > >>> There are other cases where skb's are not freed right away.
> > > >>> --
> > > >>> 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
> > > >>
> > > >> Hi Stephen:
> > > >>
> > > >> Do you mean registering a skb->destructor for pktgen then set and check
> > > >> bits in skb->tx_flag?
> > > >
> > > > Yes. Register a destructor that does something like update a counter
> > > > (number of packets pending), then just spin while number of packets
> > > > pending is over threshold.
> > > > --
> > >
> > > Not sure this is the best method, since pktgen was used to test the tx
> > > process of the device driver and NIC. If we use skb_orhpan(), we would
> > > miss the test of tx completion part.
> >
> > There are other places that delay freeing and your solution would mean
> > finding and fixing all those. Code that does that already has to use
> > skb_orphan() to work, and I was looking for a way that could use that.
> > Introducing another flag value seems like a long term burden.
> >
>
> Get the point, will draft another version.
>
> > Alternatively, virtio could do cleanup more aggressively. Maybe in response
> > to ring getting half full, or add a cleanup timer or something to avoid the
> > problem.
Timer would prevent complete deadlock but it is very expensive
in the virt scenario.
pulling at ring half full would only help if ring gets half full :)
which it does not have to.
>
> May worth to try. Another method is that virtio has a feature to notify guest
> when tx ring is empty, we could free the old tx skbs there.
> But it may brings
> extra overhead. If we could let virtio_net free the old tx skb timely, it
> would be easier to bring BQL support to virtio_net also.
>
> Thanks
We used to use notify on empty - it's still very slow.
> >
> >
> >
> > --
> > 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
* [PATCH] mISDN: improve bitops usage
From: Akinobu Mita @ 2012-11-29 11:27 UTC (permalink / raw)
To: netdev; +Cc: Akinobu Mita, Karsten Keil
This improves bitops usages in several points:
- Convert u64 to a proper bitmap declaration. This enables to remove
superfluous typecasting from 'u64' to 'unsigned long *'.
- Convert superfluous atomic bitops to non atomic bitops. The bitmap
is allocated on the stack and it is not accessed by any other threads,
so using atomic bitops is not necessary.
- Use find_next_zero_bit and find_next_zero_bit instead of calling
test_bit() for each bit.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Karsten Keil <isdn@linux-pingi.de>
Cc: netdev@vger.kernel.org
---
drivers/isdn/mISDN/tei.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/isdn/mISDN/tei.c b/drivers/isdn/mISDN/tei.c
index be88728..592f597 100644
--- a/drivers/isdn/mISDN/tei.c
+++ b/drivers/isdn/mISDN/tei.c
@@ -250,7 +250,7 @@ tei_debug(struct FsmInst *fi, char *fmt, ...)
static int
get_free_id(struct manager *mgr)
{
- u64 ids = 0;
+ DECLARE_BITMAP(ids, 64) = { [0 ... BITS_TO_LONGS(64) - 1] = 0 };
int i;
struct layer2 *l2;
@@ -261,11 +261,11 @@ get_free_id(struct manager *mgr)
__func__);
return -EBUSY;
}
- test_and_set_bit(l2->ch.nr, (u_long *)&ids);
+ __set_bit(l2->ch.nr, ids);
}
- for (i = 1; i < 64; i++)
- if (!test_bit(i, (u_long *)&ids))
- return i;
+ i = find_next_zero_bit(ids, 64, 1);
+ if (i < 64)
+ return i;
printk(KERN_WARNING "%s: more as 63 layer2 for one device\n",
__func__);
return -EBUSY;
@@ -274,7 +274,7 @@ get_free_id(struct manager *mgr)
static int
get_free_tei(struct manager *mgr)
{
- u64 ids = 0;
+ DECLARE_BITMAP(ids, 64) = { [0 ... BITS_TO_LONGS(64) - 1] = 0 };
int i;
struct layer2 *l2;
@@ -288,11 +288,11 @@ get_free_tei(struct manager *mgr)
continue;
i -= 64;
- test_and_set_bit(i, (u_long *)&ids);
+ __set_bit(i, ids);
}
- for (i = 0; i < 64; i++)
- if (!test_bit(i, (u_long *)&ids))
- return i + 64;
+ i = find_first_zero_bit(ids, 64);
+ if (i < 64)
+ return i + 64;
printk(KERN_WARNING "%s: more as 63 dynamic tei for one device\n",
__func__);
return -1;
--
1.7.11.7
^ permalink raw reply related
* Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc
From: Krzysztof Mazur @ 2012-11-29 10:57 UTC (permalink / raw)
To: David Woodhouse
Cc: David Laight, chas williams - CONTRACTOR, davem, netdev,
linux-kernel, nathan
In-Reply-To: <1354141115.21562.101.camel@shinybook.infradead.org>
On Wed, Nov 28, 2012 at 10:18:35PM +0000, David Woodhouse wrote:
> On Wed, 2012-11-28 at 09:21 +0000, David Laight wrote:
> > Even when it might make sense to sleep in close until tx drains
> > there needs to be a finite timeout before it become abortive.
>
> You are, of course, right. We should never wait for hardware for ever.
> And just to serve me right, I seem to have hit a bug in the latest Solos
> firmware (1.11) which makes it sometimes lock up when I reboot. So it
> never responds to the PKT_PCLOSE packet... and thus it deadlocks when I
> try to kill pppd and unload the module to reset it :)
>
> New version...
>
> From 53dd01c08fec5b26006a009b25e4210127fdb27a Mon Sep 17 00:00:00 2001
> From: David Woodhouse <David.Woodhouse@intel.com>
> Date: Tue, 27 Nov 2012 23:49:24 +0000
> Subject: [PATCH] solos-pci: Wait for pending TX to complete when releasing
> vcc
>
> We should no longer be calling the old pop routine for the vcc, after
> vcc_release() has completed. Make sure we wait for any pending TX skbs
> to complete, by waiting for our own PKT_PCLOSE control skb to be sent.
>
> Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
> ---
> drivers/atm/solos-pci.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/atm/solos-pci.c b/drivers/atm/solos-pci.c
> index 9851093..3720670 100644
> --- a/drivers/atm/solos-pci.c
> +++ b/drivers/atm/solos-pci.c
> @@ -92,6 +92,7 @@ struct pkt_hdr {
> };
>
> struct solos_skb_cb {
> + struct completion c;
> struct atm_vcc *vcc;
> uint32_t dma_addr;
> };
> @@ -881,11 +882,18 @@ static void pclose(struct atm_vcc *vcc)
> header->vci = cpu_to_le16(vcc->vci);
> header->type = cpu_to_le16(PKT_PCLOSE);
>
> + init_completion(&SKB_CB(skb)->c);
> +
> fpga_queue(card, SOLOS_CHAN(vcc->dev), skb, NULL);
>
> clear_bit(ATM_VF_ADDR, &vcc->flags);
> clear_bit(ATM_VF_READY, &vcc->flags);
>
> + if (!wait_for_completion_timeout(&SKB_CB(skb)->c,
> + jiffies + msecs_to_jiffies(5000)))
> + dev_warn(&card->dev->dev, "Timeout waiting for VCC close on port %d\n",
> + SOLOS_CHAN(vcc->dev));
> +
do we really need to wait here?
Why don't just do something like that:
tasklet_disable(&card->tlet);
spin_lock(&card->tx_queue_lock);
for each skb in queue
SKB_CB(skb)->vcc = NULL;
spin_unlock(&card->tx_queue_lock);
tasklet_enable(&card->tlet);
or if we really want to call vcc->pop() for such skbs:
tasklet_disable(&card->tlet);
spin_lock(&card->tx_queue_lock);
for each skb in queue {
skb_get(skb);
solos_pop(SKB_CB(skb)->vcc, skb);
SKB_CB(skb)->vcc = NULL;
}
spin_unlock(&card->tx_queue_lock);
tasklet_enable(&card->tlet);
Krzysiek
^ permalink raw reply
* Re: [net-next RFC] pktgen: don't wait for the device who doesn't free skb immediately after sent
From: Jason Wang @ 2012-11-29 10:13 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: mst, netdev, linux-kernel, virtualization, davem
In-Reply-To: <20121128085305.2b0503ce@nehalam.linuxnetplumber.net>
On Wednesday, November 28, 2012 08:53:05 AM Stephen Hemminger wrote:
> On Wed, 28 Nov 2012 14:48:52 +0800
>
> Jason Wang <jasowang@redhat.com> wrote:
> > On 11/28/2012 12:49 AM, Stephen Hemminger wrote:
> > > On Tue, 27 Nov 2012 14:45:13 +0800
> > >
> > > Jason Wang <jasowang@redhat.com> wrote:
> > >> On 11/27/2012 01:37 AM, Stephen Hemminger wrote:
> > >>> On Mon, 26 Nov 2012 15:56:52 +0800
> > >>>
> > >>> Jason Wang <jasowang@redhat.com> wrote:
> > >>>> Some deivces do not free the old tx skbs immediately after it has
> > >>>> been sent
> > >>>> (usually in tx interrupt). One such example is virtio-net which
> > >>>> optimizes for virt and only free the possible old tx skbs during the
> > >>>> next packet sending. This would lead the pktgen to wait forever in
> > >>>> the refcount of the skb if no other pakcet will be sent afterwards.
> > >>>>
> > >>>> Solving this issue by introducing a new flag IFF_TX_SKB_FREE_DELAY
> > >>>> which could notify the pktgen that the device does not free skb
> > >>>> immediately after it has been sent and let it not to wait for the
> > >>>> refcount to be one.
> > >>>>
> > >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> > >>>
> > >>> Another alternative would be using skb_orphan() and skb->destructor.
> > >>> There are other cases where skb's are not freed right away.
> > >>> --
> > >>> 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
> > >>
> > >> Hi Stephen:
> > >>
> > >> Do you mean registering a skb->destructor for pktgen then set and check
> > >> bits in skb->tx_flag?
> > >
> > > Yes. Register a destructor that does something like update a counter
> > > (number of packets pending), then just spin while number of packets
> > > pending is over threshold.
> > > --
> >
> > Not sure this is the best method, since pktgen was used to test the tx
> > process of the device driver and NIC. If we use skb_orhpan(), we would
> > miss the test of tx completion part.
>
> There are other places that delay freeing and your solution would mean
> finding and fixing all those. Code that does that already has to use
> skb_orphan() to work, and I was looking for a way that could use that.
> Introducing another flag value seems like a long term burden.
>
Get the point, will draft another version.
> Alternatively, virtio could do cleanup more aggressively. Maybe in response
> to ring getting half full, or add a cleanup timer or something to avoid the
> problem.
May worth to try. Another method is that virtio has a feature to notify guest
when tx ring is empty, we could free the old tx skbs there. But it may brings
extra overhead. If we could let virtio_net free the old tx skb timely, it
would be easier to bring BQL support to virtio_net also.
Thanks
>
>
>
> --
> 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
* [PATCH] ipv6: unify logic evaluating inet6_dev's accept_ra property
From: Shmulik Ladkani @ 2012-11-29 9:26 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Hideaki YOSHIFUJI, Thomas Graf, Tore Anderson,
shmulik.ladkani
As of 026359b [ipv6: Send ICMPv6 RSes only when RAs are accepted], the
logic determining whether to send Router Solicitations is identical
to the logic determining whether kernel accepts Router Advertisements.
However the condition itself is repeated in several code locations.
Unify it by introducing 'ipv6_accept_ra()' accessor.
Also, simplify the condition expression, making it more readable.
No semantic change.
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
Not sure about the proper header to place the 'ipv6_accept_ra' accessor.
Currently placed in include/net/ipv6.h.
Other candidates were if_inet6.h and addrconf.h.
include/net/ipv6.h | 10 ++++++++++
net/ipv6/addrconf.c | 3 +--
net/ipv6/ndisc.c | 16 ++--------------
3 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index 979bf6c..cd15585 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -271,6 +271,16 @@ struct ipv6_txoptions *ipv6_fixup_options(struct ipv6_txoptions *opt_space,
extern bool ipv6_opt_accepted(const struct sock *sk, const struct sk_buff *skb);
+static inline bool ipv6_accept_ra(struct inet6_dev *idev)
+{
+ /*
+ * If forwarding is enabled, RA are not accepted unless the special
+ * hybrid mode (accept_ra=2) is enabled.
+ */
+ return idev->cnf.forwarding ? idev->cnf.accept_ra == 2 :
+ idev->cnf.accept_ra;
+}
+
#if IS_ENABLED(CONFIG_IPV6)
static inline int ip6_frag_nqueues(struct net *net)
{
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 0424e4e..ca1ed8a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3005,8 +3005,7 @@ static void addrconf_dad_completed(struct inet6_ifaddr *ifp)
router advertisements, start sending router solicitations.
*/
- if (((ifp->idev->cnf.accept_ra == 1 && !ifp->idev->cnf.forwarding) ||
- ifp->idev->cnf.accept_ra == 2) &&
+ if (ipv6_accept_ra(ifp->idev) &&
ifp->idev->cnf.rtr_solicits > 0 &&
(dev->flags&IFF_LOOPBACK) == 0 &&
(ipv6_addr_type(&ifp->addr) & IPV6_ADDR_LINKLOCAL)) {
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 2edce30..980cdc3 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1033,18 +1033,6 @@ errout:
rtnl_set_sk_err(net, RTNLGRP_ND_USEROPT, err);
}
-static inline int accept_ra(struct inet6_dev *in6_dev)
-{
- /*
- * If forwarding is enabled, RA are not accepted unless the special
- * hybrid mode (accept_ra=2) is enabled.
- */
- if (in6_dev->cnf.forwarding && in6_dev->cnf.accept_ra < 2)
- return 0;
-
- return in6_dev->cnf.accept_ra;
-}
-
static void ndisc_router_discovery(struct sk_buff *skb)
{
struct ra_msg *ra_msg = (struct ra_msg *)skb_transport_header(skb);
@@ -1092,7 +1080,7 @@ static void ndisc_router_discovery(struct sk_buff *skb)
return;
}
- if (!accept_ra(in6_dev))
+ if (!ipv6_accept_ra(in6_dev))
goto skip_linkparms;
#ifdef CONFIG_IPV6_NDISC_NODETYPE
@@ -1248,7 +1236,7 @@ skip_linkparms:
NEIGH_UPDATE_F_ISROUTER);
}
- if (!accept_ra(in6_dev))
+ if (!ipv6_accept_ra(in6_dev))
goto out;
#ifdef CONFIG_IPV6_ROUTE_INFO
--
1.7.9
^ permalink raw reply related
* Re: [PATCH 1/1] Introduce notification events for routing changes
From: Jozsef Kadlecsik @ 2012-11-29 9:05 UTC (permalink / raw)
To: David Miller; +Cc: netdev, netfilter-devel
In-Reply-To: <20121128.175929.618012822860698468.davem@davemloft.net>
On Wed, 28 Nov 2012, David Miller wrote:
> > The netfilter MASQUERADE target does not handle the case when the routing
> > changes and the source address of existing connections become invalid.
> > The problem can be solved if routing modifications create events to which
> > the MASQUERADE target can subscribe and then delete the affected
> > connections.
> >
> > The patch adds the required event support for IPv4/IPv6.
> >
> > Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
>
> What part of the information are you actually interested in?
>
> Because just saying that a route is added or removed using fib_info X
> doesn't tell you a whole lot.
[...]
Please discard my patch. I found a simpler and more efficient way to solve
the issue in netfilter itself.
Best regards,
Jozsef
-
E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
H-1525 Budapest 114, POB. 49, Hungary
^ permalink raw reply
* RDS: sendto() with very large buffer triggering WARNING: at mm/page_alloc.c:2403
From: Tommi Rantala @ 2012-11-29 8:39 UTC (permalink / raw)
To: netdev, Venkat Venkatsubra, rds-devel; +Cc: Dave Jones
Hello,
Is RDS supposed to cap the sendto() buffer size? Saw the WARNING while
fuzzing with Trinity.
#include <string.h>
#include <arpa/inet.h>
#include <sys/socket.h>
static const char buf[1234000567];
int main(void)
{
int fd;
struct sockaddr_in sa;
fd = socket(21 /* AF_RDS */, SOCK_SEQPACKET, 0);
if (fd < 0)
return 1;
memset(&sa, 0, sizeof(sa));
sa.sin_family = AF_INET;
sa.sin_addr.s_addr = inet_addr("127.0.0.1");
sa.sin_port = htons(11111);
bind(fd, (struct sockaddr *)&sa, sizeof(sa));
sendto(fd, buf, sizeof(buf), 0, (struct sockaddr *)&sa, sizeof(sa));
return 0;
}
$ strace -e sendto ./rds-sendto
sendto(3, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"...,
1234000567, 0, {sa_family=AF_INET, sin_port=htons(11111),
sin_addr=inet_addr("127.0.0.1")}, 16) = -1 ENOMEM (Cannot allocate
memory)
[ 7421.592595] ------------[ cut here ]------------
[ 7421.592621] WARNING: at mm/page_alloc.c:2403
__alloc_pages_nodemask+0x2c0/0x9f0()
[ 7421.592628] Hardware name: EB1012
[ 7421.592633] Modules linked in:
[ 7421.592645] Pid: 3082, comm: rds-sendto Not tainted 3.7.0-rc7+ #58
[ 7421.592650] Call Trace:
[ 7421.592667] [<ffffffff810a197b>] warn_slowpath_common+0x7b/0xc0
[ 7421.592678] [<ffffffff810a19d5>] warn_slowpath_null+0x15/0x20
[ 7421.592689] [<ffffffff81171900>] __alloc_pages_nodemask+0x2c0/0x9f0
[ 7421.592700] [<ffffffff81107c90>] ? __lock_acquire+0x3a0/0x9f0
[ 7421.592711] [<ffffffff81107c90>] ? __lock_acquire+0x3a0/0x9f0
[ 7421.592724] [<ffffffff811ac6bf>] alloc_pages_current+0x7f/0xf0
[ 7421.592735] [<ffffffff8116cc19>] __get_free_pages+0x9/0x40
[ 7421.592746] [<ffffffff811b3d8a>] kmalloc_order_trace+0x3a/0x190
[ 7421.592755] [<ffffffff81107c90>] ? __lock_acquire+0x3a0/0x9f0
[ 7421.592765] [<ffffffff811b4f59>] __kmalloc+0x229/0x240
[ 7421.592778] [<ffffffff81d1b06e>] rds_message_alloc+0x1e/0xa0
[ 7421.592789] [<ffffffff81d1db66>] rds_sendmsg+0x196/0x720
[ 7421.592802] [<ffffffff81a72a80>] ? sock_update_classid+0xf0/0x2b0
[ 7421.592813] [<ffffffff81a6abec>] sock_sendmsg+0xdc/0xf0
[ 7421.592828] [<ffffffff8118e9e5>] ? might_fault+0x85/0x90
[ 7421.592838] [<ffffffff8118e99c>] ? might_fault+0x3c/0x90
[ 7421.592848] [<ffffffff81a6e0fa>] sys_sendto+0xfa/0x130
[ 7421.592859] [<ffffffff8110953d>] ? trace_hardirqs_on_caller+0x10d/0x1a0
[ 7421.592868] [<ffffffff811095dd>] ? trace_hardirqs_on+0xd/0x10
[ 7421.592881] [<ffffffff81e94c9d>] ? _raw_spin_unlock_irq+0x3d/0x70
[ 7421.592892] [<ffffffff810bab44>] ? ptrace_notify+0x74/0x90
[ 7421.592904] [<ffffffff81e96250>] tracesys+0xdd/0xe2
[ 7421.592911] ---[ end trace d9d681d0d60abf69 ]---
Tommi
^ permalink raw reply
* [PATCH: net-next] core: make GRO methods static.
From: Rami Rosen @ 2012-11-29 7:55 UTC (permalink / raw)
To: davem; +Cc: netdev, Rami Rosen
This patch changes three methods to be static and removes their
EXPORT_SYMBOLs in core/dev.c and their external declaration in
netdevice.h. The methods, dev_gro_receive(), napi_frags_finish() and
napi_skb_finish(), which are in the GRO rx path, are not used
outside core/dev.c.
Signed-off-by: Rami Rosen <ramirose@gmail.com>
---
include/linux/netdevice.h | 6 ------
net/core/dev.c | 9 +++------
2 files changed, 3 insertions(+), 12 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e9929ab..18c5dc9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2153,16 +2153,10 @@ extern void dev_kfree_skb_any(struct sk_buff *skb);
extern int netif_rx(struct sk_buff *skb);
extern int netif_rx_ni(struct sk_buff *skb);
extern int netif_receive_skb(struct sk_buff *skb);
-extern gro_result_t dev_gro_receive(struct napi_struct *napi,
- struct sk_buff *skb);
-extern gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb);
extern gro_result_t napi_gro_receive(struct napi_struct *napi,
struct sk_buff *skb);
extern void napi_gro_flush(struct napi_struct *napi, bool flush_old);
extern struct sk_buff * napi_get_frags(struct napi_struct *napi);
-extern gro_result_t napi_frags_finish(struct napi_struct *napi,
- struct sk_buff *skb,
- gro_result_t ret);
extern gro_result_t napi_gro_frags(struct napi_struct *napi);
static inline void napi_free_frags(struct napi_struct *napi)
diff --git a/net/core/dev.c b/net/core/dev.c
index 2a5f5586..2f94df2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3592,7 +3592,7 @@ void napi_gro_flush(struct napi_struct *napi, bool flush_old)
}
EXPORT_SYMBOL(napi_gro_flush);
-enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
+static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
{
struct sk_buff **pp = NULL;
struct packet_offload *ptype;
@@ -3683,7 +3683,6 @@ normal:
ret = GRO_NORMAL;
goto pull;
}
-EXPORT_SYMBOL(dev_gro_receive);
static inline gro_result_t
__napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
@@ -3710,7 +3709,7 @@ __napi_gro_receive(struct napi_struct *napi, struct sk_buff *skb)
return dev_gro_receive(napi, skb);
}
-gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
+static gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
{
switch (ret) {
case GRO_NORMAL:
@@ -3736,7 +3735,6 @@ gro_result_t napi_skb_finish(gro_result_t ret, struct sk_buff *skb)
return ret;
}
-EXPORT_SYMBOL(napi_skb_finish);
static void skb_gro_reset_offset(struct sk_buff *skb)
{
@@ -3788,7 +3786,7 @@ struct sk_buff *napi_get_frags(struct napi_struct *napi)
}
EXPORT_SYMBOL(napi_get_frags);
-gro_result_t napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
+static gro_result_t napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
gro_result_t ret)
{
switch (ret) {
@@ -3813,7 +3811,6 @@ gro_result_t napi_frags_finish(struct napi_struct *napi, struct sk_buff *skb,
return ret;
}
-EXPORT_SYMBOL(napi_frags_finish);
static struct sk_buff *napi_frags_skb(struct napi_struct *napi)
{
--
1.7.11.7
^ permalink raw reply related
* [PATCH 1/2] of_mdio: Honour "status=disabled" property of device
From: Alexander Sverdlin @ 2012-11-29 7:45 UTC (permalink / raw)
To: Stephen Warren, devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
Rob Herring, Grant Likely
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Barry.Song-kQvG35nSl+M,
w.sang-bIcnvbaLZ9MEGnE8C9+IrQ, alexander sverdlin
From: Alexander Sverdlin <alexander.sverdlin-uSbOeAmDUekAvxtiuMwx3w@public.gmane.org>
of_mdio: Honour "status=disabled" property of device
Currently of_mdiobus_register() function registers all PHY devices,
independetly from their status property in device tree. According to
"ePAPR 1.1" spec, device should only be registered if there is no
"status" property, or it has "ok" (or "okay") value (see
of_device_is_available()). In case of "platform devices",
of_platform_device_create_pdata() checks for "status" and ensures
that disabled devices are not pupulated. But such check for MDIO buses
was missing until now. Fix it.
Signed-off-by: Alexander Sverdlin <alexander.sverdlin-uSbOeAmDUekAvxtiuMwx3w@public.gmane.org>
---
--- linux.orig/drivers/of/of_mdio.c
+++ linux/drivers/of/of_mdio.c
@@ -53,7 +53,7 @@ int of_mdiobus_register(struct mii_bus *
return rc;
/* Loop over the child nodes and register a phy_device for each one */
- for_each_child_of_node(np, child) {
+ for_each_available_child_of_node(np, child) {
const __be32 *paddr;
u32 addr;
int len;
^ permalink raw reply
* Re: "tuntap: multiqueue support" causes udev fork bombs
From: Jason Wang @ 2012-11-29 5:47 UTC (permalink / raw)
To: Jiri Slaby
Cc: David S. Miller, ML netdev, LKML, rmilasan, Jiri Slaby, maxk,
vtun
In-Reply-To: <50B5E6A5.6040009@suse.cz>
On Wednesday, November 28, 2012 11:25:41 AM Jiri Slaby wrote:
> Hi,
>
> with this commit:
> commit c8d68e6be1c3b242f1c598595830890b65cea64a
> Author: Jason Wang <jasowang@redhat.com>
> Date: Wed Oct 31 19:46:00 2012 +0000
>
> tuntap: multiqueue support
>
>
> I see fork bombs from udev. It is trying to create 2048 processes. 1024
> for tx, 1024 for rx. OOM killer indeed steps in and kills everything.
Hi, thanks for the reporting, could you pls try the following patch?
---
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b44d7b7..cc3f878 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -492,9 +492,6 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
tun_set_real_num_queues(tun);
- if (tun->numqueues == 1)
- netif_carrier_on(tun->dev);
-
/* device is allowed to go away first, so no need to hold extra
* refcnt.
*/
@@ -1611,6 +1608,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
TUN_USER_FEATURES;
dev->features = dev->hw_features;
+ err = tun_attach(tun, file);
+ if (err < 0)
+ goto err_free_dev;
+
err = register_netdevice(tun->dev);
if (err < 0)
goto err_free_dev;
@@ -1620,9 +1621,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
device_create_file(&tun->dev->dev, &dev_attr_group))
pr_err("Failed to create tun sysfs files\n");
- err = tun_attach(tun, file);
- if (err < 0)
- goto err_free_dev;
+ netif_carrier_on(tun->dev);
}
tun_debug(KERN_INFO, tun, "tun_set_iff\n");
^ permalink raw reply related
* Re: [PATCH] smsc95xx: fix suspend buffer overflow
From: Joe Perches @ 2012-11-29 4:54 UTC (permalink / raw)
To: Steve Glendinning; +Cc: Steve Glendinning, netdev, Dan Carpenter
In-Reply-To: <CAKh2mn4pJG-F4zxpBj3MmRSNSV1Gwnxjw=kvfD0xqqYxErdEHg@mail.gmail.com>
On Wed, 2012-11-28 at 18:06 +0000, Steve Glendinning wrote:
> that filter code isn't pretty! If you have time
> to knock up a patch I'd be happy to test it.
Looking a bit at the code, I don't know how it's supposed to work.
This function seems broken:
static u16 smsc_crc(const u8 *buffer, size_t len, int filter)
{
return bitrev16(crc16(0xFFFF, buffer, len)) << ((filter % 2) * 16);
}
It always returns 0 when filter is odd.
I imagine 2 things:
o It should return u32
o when multiple WAKE_<foo> flags are set,
the code doesn't work properly.
^ permalink raw reply
* Re: VPN traffic leaks in IPv6/IPv4 dual-stack networks/hosts
From: Fernando Gont @ 2012-11-29 4:38 UTC (permalink / raw)
To: Jan Engelhardt; +Cc: netdev
In-Reply-To: <alpine.LNX.2.01.1211290413330.31436@nerf07.vanv.qr>
On 11/29/2012 12:15 AM, Jan Engelhardt wrote:
>
> On Thursday 2012-11-29 03:29, Fernando Gont wrote:
>>
>> Not to mention that nowadays, you will miss virtually nothing on the
>> Internet by having v6 off.
>
> Don't judge others by your own standards..
>
You don't know me enough to make claims about my standards.
Whenever you please, please come up with a list of services (by domain
name) that are not available on v4, then talk.
And, in any case, what I mentioned still holds true: preventing VPN
leaks follows the principle of "least surprise".
I said what I had to say...
Cheers,
--
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@si6networks.com
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1
^ permalink raw reply
* Re: VPN traffic leaks in IPv6/IPv4 dual-stack networks/hosts
From: Jan Engelhardt @ 2012-11-29 3:15 UTC (permalink / raw)
To: Fernando Gont; +Cc: netdev
In-Reply-To: <50B6C8A5.2090404@gont.com.ar>
On Thursday 2012-11-29 03:29, Fernando Gont wrote:
>
>Not to mention that nowadays, you will miss virtually nothing on the
>Internet by having v6 off.
Don't judge others by your own standards..
^ 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