Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v7 net-next 1/4] sh_eth: Use bool as return type of sh_eth_is_gether()
From: Sergei Shtylyov @ 2014-01-17 20:25 UTC (permalink / raw)
  To: Simon Horman, David S. Miller, netdev, linux-sh
  Cc: linux-arm-kernel, Magnus Damm
In-Reply-To: <1389918150-19058-2-git-send-email-horms+renesas@verge.net.au>

Hello.

On 01/17/2014 03:22 AM, Simon Horman wrote:

> Return a boolean from sh_eth_is_gether() and refactor it as a one-liner.

> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

Acked-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

WBR, Sergei

^ permalink raw reply

* Re: linux-next: Tree for Jan 14 (lowpan, 802.15.4)
From: Randy Dunlap @ 2014-01-17 19:20 UTC (permalink / raw)
  To: Stephen Warren, Dmitry Eremin-Solenikov, David S. Miller,
	Marcel Holtmann
  Cc: Stephen Rothwell, linux-next, kernel list, linux-zigbee-devel,
	Alexander Smirnov, netdev@vger.kernel.org, Jukka Rissanen
In-Reply-To: <52D980ED.6000603@wwwdotorg.org>

On 01/17/2014 11:13 AM, Stephen Warren wrote:
> On 01/14/2014 03:54 PM, Dmitry Eremin-Solenikov wrote:
>> Hello,
>>
>>
>> On Tue, Jan 14, 2014 at 9:49 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>>
>>> On 01/13/2014 09:51 PM, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>>
>>>> Changes since 20140113:
>>>>
>>>
>>>
>>> on i386:
>>>
>>> net/built-in.o: In function `header_create':
>>> 6lowpan.c:(.text+0x166149): undefined reference to `lowpan_header_compress'
>>> net/built-in.o: In function `bt_6lowpan_recv':
>>> (.text+0x166b3c): undefined reference to `lowpan_process_data'
>>
>> Ah, nice Makefile hack there.
>> David, Marcel, could you please consider the attached patch.
> 
> I think you forgot to "git add net/bluetooth/Makefile" into that patch;
> don't you need the following too (I certainly do, to build next-20140117)

Yes, that fixes the problems.

>> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
>> index cc6827e2ce68..80cb215826e8 100644
>> --- a/net/bluetooth/Makefile
>> +++ b/net/bluetooth/Makefile
>> @@ -12,8 +12,4 @@ bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
>>  	hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o sco.o lib.o \
>>  	a2mp.o amp.o 6lowpan.o
>>  
>> -ifeq ($(CONFIG_IEEE802154_6LOWPAN),)
>> -  bluetooth-y +=  ../ieee802154/6lowpan_iphc.o
>> -endif
>> -
>>  subdir-ccflags-y += -D__CHECK_ENDIAN__
> 
> Should I send this as a separate followup patch?
> --

I think so.  It was not part of the posted patch.

Thanks.

-- 
~Randy

^ permalink raw reply

* Re: linux-next: Tree for Jan 14 (lowpan, 802.15.4)
From: Stephen Warren @ 2014-01-17 19:13 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov, Randy Dunlap, David S. Miller,
	Marcel Holtmann
  Cc: Stephen Rothwell, linux-next, kernel list, linux-zigbee-devel,
	Alexander Smirnov, netdev@vger.kernel.org, Jukka Rissanen
In-Reply-To: <CALT56yPzfkz7=WdT0p6EYXdsQJUT+Ld9gdd72z6_czn0YaKUWA@mail.gmail.com>

On 01/14/2014 03:54 PM, Dmitry Eremin-Solenikov wrote:
> Hello,
> 
> 
> On Tue, Jan 14, 2014 at 9:49 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 01/13/2014 09:51 PM, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>
>>> Changes since 20140113:
>>>
>>
>>
>> on i386:
>>
>> net/built-in.o: In function `header_create':
>> 6lowpan.c:(.text+0x166149): undefined reference to `lowpan_header_compress'
>> net/built-in.o: In function `bt_6lowpan_recv':
>> (.text+0x166b3c): undefined reference to `lowpan_process_data'
> 
> Ah, nice Makefile hack there.
> David, Marcel, could you please consider the attached patch.

I think you forgot to "git add net/bluetooth/Makefile" into that patch;
don't you need the following too (I certainly do, to build next-20140117)

> diff --git a/net/bluetooth/Makefile b/net/bluetooth/Makefile
> index cc6827e2ce68..80cb215826e8 100644
> --- a/net/bluetooth/Makefile
> +++ b/net/bluetooth/Makefile
> @@ -12,8 +12,4 @@ bluetooth-y := af_bluetooth.o hci_core.o hci_conn.o hci_event.o mgmt.o \
>  	hci_sock.o hci_sysfs.o l2cap_core.o l2cap_sock.o smp.o sco.o lib.o \
>  	a2mp.o amp.o 6lowpan.o
>  
> -ifeq ($(CONFIG_IEEE802154_6LOWPAN),)
> -  bluetooth-y +=  ../ieee802154/6lowpan_iphc.o
> -endif
> -
>  subdir-ccflags-y += -D__CHECK_ENDIAN__

Should I send this as a separate followup patch?

^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: add trim helper and convert users
From: Eric Dumazet @ 2014-01-17 19:08 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Hannes Frederic Sowa, netdev, Daniel Borkmann, Jakub Zawadzki,
	linux-kernel
In-Reply-To: <1389982549.27141.15.camel@bwh-desktop.uk.level5networks.com>

On Fri, 2014-01-17 at 18:15 +0000, Ben Hutchings wrote:
> On Fri, 2014-01-17 at 01:28 +0100, Hannes Frederic Sowa wrote:
> [...]
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -193,6 +193,25 @@ extern int _cond_resched(void);
> >  		(__x < 0) ? -__x : __x;		\
> >  	})
> >  
> > +/**
> > + * trim - perform a reciprocal multiplication in order to "clamp" a
> > + *        value into range [0, ep_ro), where the upper interval
> > + *        endpoint is right-open. This is useful, e.g. for accessing
> > + *        a index of an array containing ep_ro elements, for example.
> > + *        Think of it as sort of modulus, only that the result isn't
> > + *        that of modulo. ;)
> > + *        More: http://homepage.cs.uiowa.edu/~jones/bcd/divide.html
> [...]
> 
> And you think trim() is an obvious name for that?!
> How about: scale_u32_to_range().
> 
> Also the first physical line of a kernel-doc comment (after the name) is
> a summary which is used, for example, in the summary line on a manual
> page.  It seems like you have the summary and full description the wrong
> way round here.

BTW the 'scaling' depends on u32 value being pretty much random.

If initial input is a small value like 0 .. 1000, then trim(x, 1000)
will return 0

I liked the reciprocal name because it was really expressing the
reciprocal idea.

^ permalink raw reply

* Re: linux-next: Tree for Jan 14 (lowpan, 802.15.4)
From: Randy Dunlap @ 2014-01-17 18:49 UTC (permalink / raw)
  To: Dmitry Eremin-Solenikov, David S. Miller, Marcel Holtmann
  Cc: Stephen Rothwell, linux-next, kernel list, linux-zigbee-devel,
	Alexander Smirnov, netdev@vger.kernel.org, Jukka Rissanen
In-Reply-To: <CALT56yPzfkz7=WdT0p6EYXdsQJUT+Ld9gdd72z6_czn0YaKUWA@mail.gmail.com>

On 01/14/2014 02:54 PM, Dmitry Eremin-Solenikov wrote:
> Hello,
> 
> 
> On Tue, Jan 14, 2014 at 9:49 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>
>> On 01/13/2014 09:51 PM, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> This tree fails (more than usual) the powerpc allyesconfig build.
>>>
>>> Changes since 20140113:
>>>
>>
>>
>> on i386:
>>
>> net/built-in.o: In function `header_create':
>> 6lowpan.c:(.text+0x166149): undefined reference to `lowpan_header_compress'
>> net/built-in.o: In function `bt_6lowpan_recv':
>> (.text+0x166b3c): undefined reference to `lowpan_process_data'
> 
> Ah, nice Makefile hack there.
> David, Marcel, could you please consider the attached patch.
> 
> 

Hi,
I guess that the Makefile hack is still causing problems.
With today's linux-next (20140117), I see these build errors (or warnings
in the case of loadable modules):


when 802.15.4 and Bluetooth are built as loadable modules:

WARNING: net/ieee802154/6lowpan_iphc: 'lowpan_header_compress' exported twice. Previous export was in net/bluetooth/bluetooth.ko
WARNING: net/ieee802154/6lowpan_iphc: 'lowpan_process_data' exported twice. Previous export was in net/bluetooth/bluetooth.ko

when they are builtin to the kernel image:

net/ieee802154/built-in.o: In function `lowpan_header_compress':
(.text+0x3d00): multiple definition of `lowpan_header_compress'
net/bluetooth/built-in.o:(.text+0x2e8f0): first defined here
net/ieee802154/built-in.o: In function `lowpan_process_data':
(.text+0x3230): multiple definition of `lowpan_process_data'
net/bluetooth/built-in.o:(.text+0x2de20): first defined here


-- 
~Randy

^ permalink raw reply

* From  Dr.David Ashton
From: Dr.David Ashton @ 2014-01-17 16:41 UTC (permalink / raw)


Alkaen Dr.David Ashton
Brondesbury , North West
Lontoo Englanti

Rakas ystävä .


Olen Dr.David Ashton Brondesbury , North West London , täällä Englannissa
. Työskentelen UBS Investment Bank London Branch . Olen kirjallisesti
sinua työhuoneestani että tulee olemaanvaltava hyötyä meille molemmille .
Minun osasto, on jäsenenäkonsernin johto-ryhmä ja Chief Risk Officer (
Greater London Regional Office ) , sain tietää, hylätyssä summa £ 15
miljoonaa Great British punta ( viisitoista miljoonaa Ison- Britannian
punta ) tilille , joka kuuluu yksi meidän ulkomaisten asiakkaiden Late Mr.
Steve Allen , joka valitettavasti menetti henkensäauto-onnettomuudessa
myös hänen vaimonsa ja ainoa tytär .

Valinta sinuun yhteyttä on herättänyt alkaenmaantieteellinen luonne missä
asut , erityisesti siitä syystä,herkkyyden tämän kaupan . Pankki
virkamiehet ovat odottaneet mitään sukulaiset tulevat -up for väite ,
mutta kukaan ei ole tehnyt niin. Itse olen hävinnyt
sijoittamallasukulaisten 4 vuotta nyt , pyydän teiltä lupaa esitellä
teille kuinlähiomaiset / Will edunsaajaakuolleen tämän rahaston ,
jottaetenee tämän tilin arvo on £ 15 miljoonaa puntaa on siirretään
tilillesi kuinlähiomaiset Late Mr. Steve Allen .

Tämä maksetaan tai jakaa näitä prosenttimääriä , 60 % minulle ja 40 %
sinulle . Olen saanut kaikki tarvittavat oikeudelliset asiakirjat , joita
voidaan käyttää varmuuskopiointiin tämä väite teemme . Kaikki mitä
tarvitsee tehdä, on täyttää teidän nimetasiakirjat ja laillistaa
setuomioistuin tässä todistaa teitälaillinen edunsaajarahasto .

All I vaatia nyt on rehellinen yhteistyö , luottamuksellisuus ja Trust ,
jotta voimme näkee tämän tapahtuman kautta . Takaan , että tämä
toteutetaan allelaillinen järjestely , että sinua suojataan rikottu lakia.
Haluan teidän ymmärtävän, että olen työskennellyt tämän pankin 17 vuotta ,
ja olen voinut turvata kaikki oikeudelliset asiakirjat , jotta voit
perivät tämän rahaston

Antakaa minulle seuraavat : koska meillä on muutama päivä aikaa ajaa se
läpi tämä on erittäin kiireellinen PLEASE .


1 . Koko nimi
2 . Your suora matkapuhelinnumero
3 . Olet Yhteystiedot Osoite
4 . Syntymäaika

Mentyään läpisuunnitelmallista haku , päätin ottaa sinuun yhteyttä
toivoen, että löydät tämän paljon mielenkiintoista . Ole hyvä ja teidän
vahvistuksen tähän viestiin ja osoittaa kiinnostuksesi aion toimittaa
sinulle lisätietoja . Pyrittävä haluaisin tietää päätöksen niin pian kuin
mahdollista .


Ystävällisin terveisin,
Dr.David Ashton
Matkapuhelinnumero +447937425508

^ permalink raw reply

* Re: [PATCH net-next] net: vxlan: do not use vxlan_net before checking event type
From: Daniel Borkmann @ 2014-01-17 18:32 UTC (permalink / raw)
  To: Cong Wang
  Cc: David Miller, Linux Kernel Network Developers, Eric W. Biederman,
	Jesse Brandeburg
In-Reply-To: <CAM_iQpV4FGdbHo9UVFhN9yqhZuGEZdrynikvrTZ0YZJTPVCmLg@mail.gmail.com>

On 01/17/2014 06:30 PM, Cong Wang wrote:
> On Fri, Jan 17, 2014 at 3:55 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> -       if (event == NETDEV_UNREGISTER)
>> +       if (event == NETDEV_UNREGISTER) {
>> +               vn = net_generic(dev_net(dev), vxlan_net_id);
>>                  vxlan_handle_lowerdev_unregister(vn, dev);
>> +       }
>
> There is no need to keep vxlan_handle_lowerdev_unregister(),
> it is too short. So, just use my patch.

If you want to do cleanups, whatever, I really don't care.
You had your chance to complain about that when you reviewed
the initial version ... it has nothing to do with the fix.

^ permalink raw reply

* Re: [PATCH net-next] net: vxlan: do not use vxlan_net before checking event type
From: Jesse Brandeburg @ 2014-01-17 18:20 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, netdev, Eric W. Biederman, Cong Wang
In-Reply-To: <1389959706-30976-1-git-send-email-dborkman@redhat.com>

On Fri, 17 Jan 2014 12:55:06 +0100
Daniel Borkmann <dborkman@redhat.com> wrote:

> Jesse Brandeburg reported that commit acaf4e70997f caused a panic
> when adding a network namespace while vxlan module was present in
> the system:

I ran a quick test and the namespace issue no longer occurs once this
patch is applied.  As for this one vs Cong's, you guys can fight that
out.  Thanks for fixing this.

Tested-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

^ permalink raw reply

* Re: [PATCH net-next v2 2/3] net: add trim helper and convert users
From: Ben Hutchings @ 2014-01-17 18:15 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: netdev, Daniel Borkmann, Jakub Zawadzki, Eric Dumazet,
	linux-kernel
In-Reply-To: <1389918519-23779-3-git-send-email-hannes@stressinduktion.org>

On Fri, 2014-01-17 at 01:28 +0100, Hannes Frederic Sowa wrote:
[...]
> --- a/include/linux/kernel.h
> +++ b/include/linux/kernel.h
> @@ -193,6 +193,25 @@ extern int _cond_resched(void);
>  		(__x < 0) ? -__x : __x;		\
>  	})
>  
> +/**
> + * trim - perform a reciprocal multiplication in order to "clamp" a
> + *        value into range [0, ep_ro), where the upper interval
> + *        endpoint is right-open. This is useful, e.g. for accessing
> + *        a index of an array containing ep_ro elements, for example.
> + *        Think of it as sort of modulus, only that the result isn't
> + *        that of modulo. ;)
> + *        More: http://homepage.cs.uiowa.edu/~jones/bcd/divide.html
[...]

And you think trim() is an obvious name for that?!
How about: scale_u32_to_range().

Also the first physical line of a kernel-doc comment (after the name) is
a summary which is used, for example, in the summary line on a manual
page.  It seems like you have the summary and full description the wrong
way round here.

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

* Re: [Xen-devel] [PATCH net-next v2] xen-netfront: clean up code in xennet_release_rx_bufs
From: David Vrabel @ 2014-01-17 17:50 UTC (permalink / raw)
  To: annie li; +Cc: Wei Liu, ian.campbell, netdev, xen-devel, andrew.bennieston,
	davem
In-Reply-To: <52D94F8C.7060509@oracle.com>

On 17/01/14 15:43, annie li wrote:
> 
> No, I am trying to implement 2 patches.

I don't understand the need for two patches here, particularly when
the first patch introduces a security issue.  You can fold the following 
(untested) patch into your v2 patch and give it a try?

Thanks.

David

8<----------------------
xen-netfront: prevent unsafe reuse of rx buf pages after uninit

---
 drivers/net/xen-netfront.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 692589e..47aa599 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1134,19 +1134,32 @@ static void xennet_release_tx_bufs(struct netfront_info *np)
 
 static void xennet_release_rx_bufs(struct netfront_info *np)
 {
-	struct sk_buff *skb;
 	int id, ref;
 
 	spin_lock_bh(&np->rx_lock);
 
 	for (id = 0; id < NET_RX_RING_SIZE; id++) {
+		struct sk_buff *skb;
+		skb_frag_t *frag;
+		const struct page *page;
+
+		skb = np->rx_skbs[id];
+		if (!skb)
+			continue;
+
 		ref = np->grant_rx_ref[id];
 		if (ref == GRANT_INVALID_REF)
 			continue;
 
-		skb = np->rx_skbs[id];
-		gnttab_end_foreign_access_ref(ref, 0);
-		gnttab_release_grant_reference(&np->gref_rx_head, ref);
+		frag = &skb_shinfo(skb)->frags[0];
+		page = skb_frag_page(frag);
+
+		/* gnttab_end_foreign_access() needs a page ref until
+		 * foreign access is ended (which may be deferred).
+		 */
+		get_page(page);
+
+		gnttab_end_foreign_access(ref, 0, page);
 		np->grant_rx_ref[id] = GRANT_INVALID_REF;
 
 		kfree_skb(skb);
-- 
1.7.2.5

^ permalink raw reply related

* Re: [PATCH RFC 4/6] net: rfkill: gpio: add device tree support
From: Chen-Yu Tsai @ 2014-01-17 17:43 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-arm-kernel, Johannes Berg, David S. Miller, devicetree,
	netdev, linux-wireless, linux-sunxi, linux-kernel, Maxime Ripard
In-Reply-To: <201401171747.46332.arnd-r2nGTMty4D4@public.gmane.org>

On Sat, Jan 18, 2014 at 12:47 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Friday 17 January 2014, Chen-Yu Tsai wrote:
>> diff --git a/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt b/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt
>> new file mode 100644
>> index 0000000..8a07ea4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/rfkill/rfkill-gpio.txt
>> @@ -0,0 +1,26 @@
>> +GPIO controlled RFKILL devices
>> +
>> +Required properties:
>> +- compatible   : Must be "rfkill-gpio".
>> +- rfkill-name  : Name of RFKILL device
>> +- rfkill-type  : Type of RFKILL device: 1 for WiFi, 2 for BlueTooth
>> +- NAME_shutdown-gpios  : GPIO phandle to shutdown control
>> +                         (phandle must be the second)
>> +- NAME_reset-gpios     : GPIO phandle to reset control
>> +
>> +NAME must match the rfkill-name property. NAME_shutdown-gpios or
>> +NAME_reset-gpios, or both, must be defined.
>> +
>
> I don't understand this part. Why do you include the name in the
> gpios property, rather than just hardcoding the property strings
> to "shutdown-gpios" and "reset-gpios"?

This quirk is a result of how gpiod_get_index implements device tree
lookup. You'll also notice that the shutdown GPIO must be the second
phandle, as the driver uses indexed lookup to support ACPI cases.
If con_id is given, it is prepended to "gpios" as the property string.
con_id is also used as the label passed to gpiod_request, which is
then shown in /sys/kernel/debug/gpio.

I can do a seperate lookup for the device tree case, with or without
fallback to platform lookup tables. Then the names can be "reset-gpio"
and "shutdown-gpio". Need to promote gpiod_request to non-static so
we can register usage of the gpio, to match non-dt code path.

Personally I prefer adding a "label" parameter to gpiod_get_index, so
we can use a different name than con_id. con_id isn't used in the ACPI
case, and is optional in platform lookup case. However device tree
lookup is dependent on this. What I see is non-uniform behavior
between the three. In my opinion this is undesirable, but I haven't
come up with a good solution yet.

About the property string, is the plural form required, even though we
only want one?

Thanks!

ChenYu

> The description of hte "rfkill-name" property seems to suggest
> that you can only have one logical RFKILL device per device node,
> so he names would not be ambiguous.
>
>         Arnd

^ permalink raw reply

* [PATCH v2 net-next 0/12] bonding: add an option to rely on unvalidated arp packets
From: Veaceslav Falico @ 2014-01-17 16:56 UTC (permalink / raw)
  To: netdev
  Cc: Rob Landley, Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Nikolay Aleksandrov, Ding Tianhong, Neil Horman, Veaceslav Falico

Hi,

v1 -> v2:
Don't remove the 'all traffic' functionality - rather, add new arp_validate
options to specify that we want *only* unvalidated arps.

Currently, if arp_validate is off (0), slave_last_rx() returns the
slave->dev->last_rx, which is always updated on *any* packet received by
slave, and not only arps. This means that, if the validation of arps is
off, we're treating *any* incoming packet as a proof of slave being up, and
not only arps.

This might seem logical at the first glance, however it can cause a lot of
troubles and false-positives, one example would be:

The arp_ip_target is NOT accessible, however someone in the broadcast domain
spams with any broadcast traffic. This way bonding will be tricked that the
slave is still up (as in - can access arp_ip_target), while it's not.

The net_device->last_rx is already used in a lot of drivers (even though the
comment states to NOT do it :)), and it's also ugly to modify it from bonding.

However, some loadbalance setups might rely on the fact that even non-arp
traffic is a sign of slave being up - and we definitely can't break anyones
config - so an extension to arp_validate is needed.

So, to fix this, add an option for the user to specify if he wants to
filter out non-arp traffic on unvalidated slaves, remove the last_rx from
bonding, *always* call bond_arp_rcv() in slave's rx_handler (which is
bond_handle_frame), and if we spot an arp there with this option on - update
the slave->last_arp_rx - and use it instead of net_device->last_rx. Finally,
rename last_arp_rx to last_rx to reflect the changes.

Also rename slave->jiffies to ->last_link_up, to reflect better its
meaning, add the new option's documentation and update the arp_validate one
to be a bit more descriptive.

CC: Rob Landley <rob@landley.net>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
CC: Ding Tianhong <dingtianhong@huawei.com>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: netdev@vger.kernel.org
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 Documentation/networking/bonding.txt | 40 ++++++++++++++++++++++-----
 drivers/net/bonding/bond_main.c      | 53 ++++++++++++++++--------------------
 drivers/net/bonding/bond_options.c   | 18 ++----------
 drivers/net/bonding/bonding.h        | 26 ++++++++++++------
 include/linux/netdevice.h            |  8 +-----
 5 files changed, 77 insertions(+), 68 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next] net: vxlan: do not use vxlan_net before checking event type
From: Cong Wang @ 2014-01-17 17:30 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: David Miller, Linux Kernel Network Developers, Eric W. Biederman,
	Jesse Brandeburg
In-Reply-To: <1389959706-30976-1-git-send-email-dborkman@redhat.com>

On Fri, Jan 17, 2014 at 3:55 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> -       if (event == NETDEV_UNREGISTER)
> +       if (event == NETDEV_UNREGISTER) {
> +               vn = net_generic(dev_net(dev), vxlan_net_id);
>                 vxlan_handle_lowerdev_unregister(vn, dev);
> +       }

There is no need to keep vxlan_handle_lowerdev_unregister(),
it is too short. So, just use my patch.

^ permalink raw reply

* Re: [RFC PATCH net-next 3/3] virtio-net: Add accelerated RFS support
From: Ben Hutchings @ 2014-01-17 17:20 UTC (permalink / raw)
  To: Zhi Yong Wu
  Cc: Stefan Hajnoczi, Linux Netdev List, Tom Herbert, Eric Dumazet,
	David S. Miller, Zhi Yong Wu
In-Reply-To: <CAEH94LgKJ0qThGHS-hqXsYKqOe+ns2pRi-wfZy=af1RMejOZ5A@mail.gmail.com>

On Sat, 2014-01-18 at 00:54 +0800, Zhi Yong Wu wrote:
> On Fri, Jan 17, 2014 at 7:16 AM, Ben Hutchings
> <bhutchings@solarflare.com> wrote:
[...]
> > However, to take advantage of ARFS on a physical net driver, it would be
> > necessary to send a control request for part 2.
> aRFS on a physical net driver? What is this physical net driver? I
> thought that in order to enable aRFS, guest virtio_net driver should
> send a control request to its emulated virtio_net NIC.
[...]

If the backend is connected to a macvlan device on top of a physical net
device that supports ARFS, then there is further potential for improving
performance by steering to the best physical RX queue and CPU as well as
the best virtio_net RX queue and vCPU.

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

* Re: [PATCH net-next 0/6] bonding: only rely on arp packets if arp monitor is used
From: Veaceslav Falico @ 2014-01-17 17:07 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: netdev, Andy Gospodarek, David S. Miller
In-Reply-To: <20140117065718.GA5699@redhat.com>

On Fri, Jan 17, 2014 at 07:57:18AM +0100, Veaceslav Falico wrote:
...snip...
>What do you think about this*? It's on top of this series, extends
>arp_validate to (not) filter out ARPs on not-validated slaves and permits
>it to be used in non-AB mode (also, we don't need that bond->lock, we're
>always under RCU).

Hi Jay,

In the meanwhile I've integrated this approach (adding new arp_validate
options) and sent v2.

The new version, when used with arp_validate=0/1/2/3 (old values) behaves
exactly the same, while adding 3 new options - only unvalidated ARPs,
validated ARPs on active and unvalidated ARPs on slave and vice versa
(there's no analogue for ALL_ARP cause it doesn't make much sense - as the
validated ARPs for both slaves are already ARPs :) ).

Hope that helps, and sorry that I didn't do my homework in the first time.

Thank you!

^ permalink raw reply

* Re: Multicast packets receiving problem on linux since version 3.10.1
From: Vlad Yasevich @ 2014-01-17 17:08 UTC (permalink / raw)
  To: Andrey Dmitrov, netdev
  Cc: Konstantin Ushakov, Alexandra N. Kossovsky, Yurij Plotnikov
In-Reply-To: <52D94803.1040406@oktetlabs.ru>

On 01/17/2014 10:10 AM, Andrey Dmitrov wrote:
> Greetings,
> 
> there is a problem with receiving multicast packets on linux-3.10.1 and
> newer. It's reproducible with two hosts (host_A, host_B), with eth3@host_A
> directly connected to eth3@host_B. Two VLANs and one multicast group are
> used.
> Each side opens two sockets and binds them to different VLAN interfaces.
> host_A arranges both sockets to receive multicast packets of the group.
> Then the first socket is removed from the group. After this the second
> socket
> can no longer receive multicast packets of the group, which sent by the
> second host. See the example below.
> 


Thanks for the report.  Looks like a bug in dev_mc_sync().  The problem
appears to be that once an address is synched to eth3 from eth3.999, the
sync from eth3.1001 will fail with EEXIST and will not bump the refcount
on the address added to eth3.  Then when you leave the group, the group,
the address is removed from eth3.

Let me work up a patch.


Thanks
-vlad

> host_A:
> Two VLANs 999 and 1001 are added on eth3:
> eth3.999      10.208.14.1
> eth3.1001     10.208.15.1
> 
> 1. socket(SOCK_DGRAM) -> 3
> 2. setsockopt(3, SOL_SOCKET, SO_REUSEADDR, 1) -> 0
> 3. setsockopt(3, IPPROTO_IP, MCAST_JOIN_GROUP, {229.17.88.168,
> eth3.999}) -> 0
> 4. setsockopt(3, SOL_SOCKET, SO_BINDTODEVICE, eth3.999) -> 0
> 5. bind(3, 0.0.0.0:29214)->0
> 6. socket(SOCK_DGRAM) -> 4
> 7. setsockopt(4, SOL_SOCKET, SO_REUSEADDR, 1) -> 0
> 8. setsockopt(4, IPPROTO_IP, MCAST_JOIN_GROUP, {229.17.88.168,
> eth3.1001}) -> 0
> 9. setsockopt(4, SOL_SOCKET, SO_BINDTODEVICE, eth3.1001) -> 0
> 10. bind(4, 0.0.0.0:29214) -> 0
> 
> 11. poll({{3, POLLIN}, {4, POLLIN}}, 2, 30000) -> 2
> 12. recv(3, buf, 100) -> 100
> 13. recv(4, buf, 99) -> 99
> 14. setsockopt(3, IPPROTO_IP, MCAST_LEAVE_GROUP, {229.17.88.168,
> eth3.999}) -> 0
> 15. poll({{3, POLLIN}, {4, POLLIN}}, 2, 30000) -> 0
> 
> Socket 4 does not receive the multicast packet on linux 3.10 and newer. But
> it receives the packet with older linux versions. Probably the packet is
> filtered by NIC, tcpdump does not see it.
> 
> 
> host_B:
> Two VLANs 999 and 1001 are added on eth3:
> eth3.999      10.208.14.2
> eth3.1001     10.208.15.2
> 
> 16. socket(SOCK_DGRAM) -> 3
> 17. bind(3, 10.208.14.2:29219) -> 0
> 18. socket(SOCK_DGRAM) -> 4
> 19. bind(4, 10.208.15.2:29219) -> 0
> 20. sendto(3, 229.17.88.168:29214, buf, 100) -> 100
> 21. sendto(4, 229.17.88.168:29214, buf, 99) -> 99
> Continue when socket 3 on host_A will leave the group (line 14).
> 22. sendto(3, 229.17.88.168:29214, buf, 100) -> 100
> 23. sendto(4, 229.17.88.168:29214, buf, 99) -> 99
> 
> 
> Note, that if I replace step #14 with:
>> setsockopt(4, IPPROTO_IP, MCAST_LEAVE_GROUP, {229.17.88.168,
> eth3.1001}) -> 0
> and remove the second socket from the group (instead of the first one) -
> then
> the first socket will receive it's packet.
> 
> Find client and server C programs that reproduce the problem attached.
> The client
> logs received packets. As stated above in good case it will log 3
> packets and in
> bad one - only 2. Use the following command lines to start the client
> and server:
> host_A:
> sudo ip link add link eth3 name eth3.999 type vlan id 999
> sudo ifconfig eth3.999 10.208.14.1/24
> sudo ip link add link eth3 name eth3.1001 type vlan id 1001
> sudo ifconfig eth3.1001 10.208.15.1/24
> 
> gcc mcast_client.c -o cl
> sudo ./cl
> 
> host_B:
> sudo ip link add link eth3 name eth3.999 type vlan id 999
> sudo ifconfig eth3.999 10.208.14.2/24
> sudo ip link add link eth3 name eth3.1001 type vlan id 1001
> sudo ifconfig eth3.1001 10.208.15.2/24
> 
> gcc mcast_serv.c -o serv
> ./serv
> 
> Thanks in advance,
> Andrey Dmitrov

^ permalink raw reply

* [PATCH v2 net-next 12/12] bonding: rename last_arp_rx to last_rx
From: Veaceslav Falico @ 2014-01-17 16:59 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

To reflect the new meaning.

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 | 12 ++++++------
 drivers/net/bonding/bonding.h   |  4 ++--
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 13a3ff8..41f3109 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1444,10 +1444,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	bond_update_speed_duplex(new_slave);
 
-	new_slave->last_arp_rx = jiffies -
+	new_slave->last_rx = jiffies -
 		(msecs_to_jiffies(bond->params.arp_interval) + 1);
 	for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
-		new_slave->target_last_arp_rx[i] = new_slave->last_arp_rx;
+		new_slave->target_last_arp_rx[i] = new_slave->last_rx;
 
 	if (bond->params.miimon && !bond->params.use_carrier) {
 		link_reporting = bond_check_dev_link(bond, slave_dev, 1);
@@ -2277,7 +2277,7 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
 		pr_debug("bva: sip %pI4 not found in targets\n", &sip);
 		return;
 	}
-	slave->last_arp_rx = jiffies;
+	slave->last_rx = jiffies;
 	slave->target_last_arp_rx[i] = jiffies;
 }
 
@@ -2292,7 +2292,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 	if (!slave_do_arp_validate(bond, slave)) {
 		if ((slave_do_arp_validate_only(bond, slave) && is_arp) ||
 		    !slave_do_arp_validate_only(bond, slave))
-			slave->last_arp_rx = jiffies;
+			slave->last_rx = jiffies;
 		return RX_HANDLER_ANOTHER;
 	} else if (!is_arp) {
 		return RX_HANDLER_ANOTHER;
@@ -2404,7 +2404,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 
 		if (slave->link != BOND_LINK_UP) {
 			if (bond_time_in_interval(bond, trans_start, 1) &&
-			    bond_time_in_interval(bond, slave->last_arp_rx, 1)) {
+			    bond_time_in_interval(bond, slave->last_rx, 1)) {
 
 				slave->link  = BOND_LINK_UP;
 				bond_set_active_slave(slave);
@@ -2433,7 +2433,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 			 * if we don't know our ip yet
 			 */
 			if (!bond_time_in_interval(bond, trans_start, 2) ||
-			    !bond_time_in_interval(bond, slave->last_arp_rx, 2)) {
+			    !bond_time_in_interval(bond, slave->last_rx, 2)) {
 
 				slave->link  = BOND_LINK_DOWN;
 				bond_set_backup_slave(slave);
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 15cf6b7..3087f78 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -186,7 +186,7 @@ struct slave {
 	int    delay;
 	/* all three in jiffies */
 	unsigned long last_link_up;
-	unsigned long last_arp_rx;
+	unsigned long last_rx;
 	unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
 	s8     link;    /* one of BOND_LINK_XXXX */
 	s8     new_link;
@@ -359,7 +359,7 @@ static inline unsigned long slave_last_rx(struct bonding *bond,
 	if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
 		return slave_oldest_target_arp_rx(bond, slave);
 
-	return slave->last_arp_rx;
+	return slave->last_rx;
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 07/12] bonding: use the new options to correctly set last_arp_rx
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev
  Cc: Veaceslav Falico, Rob Landley, David S. Miller,
	Nikolay Aleksandrov, Ding Tianhong, Neil Horman
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

Now that the options are in place - arp_validate can be set to receive all
the traffic or only arp packets to verify if the slave is up, when the
slave isn't validated.

CC: Rob Landley <rob@landley.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
CC: Ding Tianhong <dingtianhong@huawei.com>
CC: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 532a452..48491d9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2290,15 +2290,16 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 	struct arphdr *arp = (struct arphdr *)skb->data;
 	unsigned char *arp_ptr;
 	__be32 sip, tip;
-	int alen;
+	int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
 
-	slave->last_arp_rx = jiffies;
-
-	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
+	if (!slave_do_arp_validate(bond, slave)) {
+		if ((slave_do_arp_validate_only(bond, slave) && is_arp) ||
+		    !slave_do_arp_validate_only(bond, slave))
+			slave->last_arp_rx = jiffies;
 		return RX_HANDLER_ANOTHER;
-
-	if (!slave_do_arp_validate(bond, slave))
-		goto out_unlock;
+	} else if (!is_arp) {
+		return RX_HANDLER_ANOTHER;
+	}
 
 	alen = arp_hdr_len(bond->dev);
 
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 10/12] bonding: remove useless updating of slave->dev->last_rx
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

Now that all the logic is handled via last_arp_rx, we don't need to use
last_rx.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_main.c | 3 ---
 include/linux/netdevice.h       | 8 +-------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index eccd407..3f95042 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1166,9 +1166,6 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 	slave = bond_slave_get_rcu(skb->dev);
 	bond = slave->bond;
 
-	if (bond->params.arp_interval)
-		slave->dev->last_rx = jiffies;
-
 	recv_probe = ACCESS_ONCE(bond->recv_probe);
 	if (recv_probe) {
 		ret = recv_probe(skb, bond, slave);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e985231..0fe8af0 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1312,13 +1312,7 @@ struct net_device {
 /*
  * Cache lines mostly used on receive path (including eth_type_trans())
  */
-	unsigned long		last_rx;	/* Time of last Rx
-						 * This should not be set in
-						 * drivers, unless really needed,
-						 * because network stack (bonding)
-						 * use it if/when necessary, to
-						 * avoid dirtying this cache line.
-						 */
+	unsigned long		last_rx;	/* Time of last Rx */
 
 	/* Interface address info used in eth_type_trans() */
 	unsigned char		*dev_addr;	/* hw address, (before bcast
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 11/12] bonding: trivial: rename slave->jiffies to ->last_link_up
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

slave->jiffies is updated every time the slave becomes active, which, for
bonding, means that its link is 'up'.

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 | 20 ++++++++++----------
 drivers/net/bonding/bonding.h   |  3 ++-
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3f95042..13a3ff8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -849,7 +849,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 		return;
 
 	if (new_active) {
-		new_active->jiffies = jiffies;
+		new_active->last_link_up = jiffies;
 
 		if (new_active->link == BOND_LINK_BACK) {
 			if (USES_PRIMARY(bond->params.mode)) {
@@ -1491,7 +1491,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	}
 
 	if (new_slave->link != BOND_LINK_DOWN)
-		new_slave->jiffies = jiffies;
+		new_slave->last_link_up = jiffies;
 	pr_debug("Initial state of slave_dev is BOND_LINK_%s\n",
 		new_slave->link == BOND_LINK_DOWN ? "DOWN" :
 			(new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
@@ -1926,7 +1926,7 @@ static int bond_miimon_inspect(struct bonding *bond)
 				 * recovered before downdelay expired
 				 */
 				slave->link = BOND_LINK_UP;
-				slave->jiffies = jiffies;
+				slave->last_link_up = jiffies;
 				pr_info("%s: link status up again after %d ms for interface %s.\n",
 					bond->dev->name,
 					(bond->params.downdelay - slave->delay) *
@@ -2001,7 +2001,7 @@ static void bond_miimon_commit(struct bonding *bond)
 
 		case BOND_LINK_UP:
 			slave->link = BOND_LINK_UP;
-			slave->jiffies = jiffies;
+			slave->last_link_up = jiffies;
 
 			if (bond->params.mode == BOND_MODE_8023AD) {
 				/* prevent it from being the active one */
@@ -2347,7 +2347,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
 		bond_validate_arp(bond, slave, sip, tip);
 	else if (bond->curr_active_slave &&
 		 time_after(slave_last_rx(bond, bond->curr_active_slave),
-			    bond->curr_active_slave->jiffies))
+			    bond->curr_active_slave->last_link_up))
 		bond_validate_arp(bond, slave, tip, sip);
 
 out_unlock:
@@ -2393,9 +2393,9 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 	oldcurrent = ACCESS_ONCE(bond->curr_active_slave);
 	/* see if any of the previous devices are up now (i.e. they have
 	 * xmt and rcv traffic). the curr_active_slave does not come into
-	 * the picture unless it is null. also, slave->jiffies is not needed
-	 * here because we send an arp on each slave and give a slave as
-	 * long as it needs to get the tx/rx within the delta.
+	 * the picture unless it is null. also, slave->last_link_up is not
+	 * needed here because we send an arp on each slave and give a slave
+	 * as long as it needs to get the tx/rx within the delta.
 	 * TODO: what about up/down delay in arp mode? it wasn't here before
 	 *       so it can wait
 	 */
@@ -2517,7 +2517,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
 		 * active.  This avoids bouncing, as the last receive
 		 * times need a full ARP monitor cycle to be updated.
 		 */
-		if (bond_time_in_interval(bond, slave->jiffies, 2))
+		if (bond_time_in_interval(bond, slave->last_link_up, 2))
 			continue;
 
 		/*
@@ -2710,7 +2710,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
 	new_slave->link = BOND_LINK_BACK;
 	bond_set_slave_active_flags(new_slave);
 	bond_arp_send_all(bond, new_slave);
-	new_slave->jiffies = jiffies;
+	new_slave->last_link_up = jiffies;
 	rcu_assign_pointer(bond->current_arp_slave, new_slave);
 }
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index d0964c0..15cf6b7 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -184,7 +184,8 @@ struct slave {
 	struct net_device *dev; /* first - useful for panic debug */
 	struct bonding *bond; /* our master */
 	int    delay;
-	unsigned long jiffies;
+	/* all three in jiffies */
+	unsigned long last_link_up;
 	unsigned long last_arp_rx;
 	unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
 	s8     link;    /* one of BOND_LINK_XXXX */
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 09/12] bonding: use last_arp_rx in bond_loadbalance_arp_mon()
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

Now that last_arp_rx correctly show the last time we've received an ARP, we
can use it safely instead of slave->dev->last_rx.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 48491d9..eccd407 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2407,7 +2407,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 
 		if (slave->link != BOND_LINK_UP) {
 			if (bond_time_in_interval(bond, trans_start, 1) &&
-			    bond_time_in_interval(bond, slave->dev->last_rx, 1)) {
+			    bond_time_in_interval(bond, slave->last_arp_rx, 1)) {
 
 				slave->link  = BOND_LINK_UP;
 				bond_set_active_slave(slave);
@@ -2436,7 +2436,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
 			 * if we don't know our ip yet
 			 */
 			if (!bond_time_in_interval(bond, trans_start, 2) ||
-			    !bond_time_in_interval(bond, slave->dev->last_rx, 2)) {
+			    !bond_time_in_interval(bond, slave->last_arp_rx, 2)) {
 
 				slave->link  = BOND_LINK_DOWN;
 				bond_set_backup_slave(slave);
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 08/12] bonding: use last_arp_rx in slave_last_rx()
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

Now that last_arp_rx really has the last time we've received any (validated or
not) ARP, we can use it in slave_last_rx() instead of slave->dev->last_rx.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bonding.h | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 1fbbf04..d0964c0 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -355,14 +355,10 @@ static inline unsigned long slave_oldest_target_arp_rx(struct bonding *bond,
 static inline unsigned long slave_last_rx(struct bonding *bond,
 					struct slave *slave)
 {
-	if (slave_do_arp_validate(bond, slave)) {
-		if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
-			return slave_oldest_target_arp_rx(bond, slave);
-		else
-			return slave->last_arp_rx;
-	}
-
-	return slave->dev->last_rx;
+	if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
+		return slave_oldest_target_arp_rx(bond, slave);
+
+	return slave->last_arp_rx;
 }
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 0/12] bonding: add an option to rely on unvalidated arp packets
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev
  Cc: Rob Landley, Jay Vosburgh, Andy Gospodarek, David S. Miller,
	Nikolay Aleksandrov, Ding Tianhong, Neil Horman, Veaceslav Falico

Hi,

v1 -> v2:
Don't remove the 'all traffic' functionality - rather, add new arp_validate
options to specify that we want *only* unvalidated arps.

Currently, if arp_validate is off (0), slave_last_rx() returns the
slave->dev->last_rx, which is always updated on *any* packet received by
slave, and not only arps. This means that, if the validation of arps is
off, we're treating *any* incoming packet as a proof of slave being up, and
not only arps.

This might seem logical at the first glance, however it can cause a lot of
troubles and false-positives, one example would be:

The arp_ip_target is NOT accessible, however someone in the broadcast domain
spams with any broadcast traffic. This way bonding will be tricked that the
slave is still up (as in - can access arp_ip_target), while it's not.

The net_device->last_rx is already used in a lot of drivers (even though the
comment states to NOT do it :)), and it's also ugly to modify it from bonding.

However, some loadbalance setups might rely on the fact that even non-arp
traffic is a sign of slave being up - and we definitely can't break anyones
config - so an extension to arp_validate is needed.

So, to fix this, add an option for the user to specify if he wants to
filter out non-arp traffic on unvalidated slaves, remove the last_rx from
bonding, *always* call bond_arp_rcv() in slave's rx_handler (which is
bond_handle_frame), and if we spot an arp there with this option on - update
the slave->last_arp_rx - and use it instead of net_device->last_rx. Finally,
rename last_arp_rx to last_rx to reflect the changes.

Also rename slave->jiffies to ->last_link_up, to reflect better its
meaning, add the new option's documentation and update the arp_validate one
to be a bit more descriptive.

CC: Rob Landley <rob@landley.net>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
CC: Ding Tianhong <dingtianhong@huawei.com>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: netdev@vger.kernel.org
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 Documentation/networking/bonding.txt | 40 ++++++++++++++++++++++-----
 drivers/net/bonding/bond_main.c      | 53 ++++++++++++++++--------------------
 drivers/net/bonding/bond_options.c   | 18 ++----------
 drivers/net/bonding/bonding.h        | 26 ++++++++++++------
 include/linux/netdevice.h            |  8 +-----
 5 files changed, 77 insertions(+), 68 deletions(-)

^ permalink raw reply

* [PATCH v2 net-next 06/12] bonding: document the new _arp options for arp_validate
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 Documentation/networking/bonding.txt | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 3620690..a0c1bca2 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -279,19 +279,45 @@ arp_validate
 
 	none or 0
 
-		No validation is performed.  This is the default.
+		No validation is performed.  This is the default. Any arriving
+		traffic (arp or non-arp) is considered a proof that the slave
+		is up.
 
 	active or 1
 
-		Validation is performed only for the active slave.
+		Validation is performed only for the active slave. Only ARPs
+		that arrive from any arp_ip_target are considered legit. The
+		backup slave still does no validation (as if arp_validate=0).
 
 	backup or 2
 
-		Validation is performed only for backup slaves.
+		Validation is performed only for backup slaves. Only ARPs
+		that arrive from any arp_ip_target are considered legit. The
+		active slave still has no validation (as if arp_validate=0).
 
 	all or 3
 
-		Validation is performed for all slaves.
+		Validation is performed for all slaves. Only ARPs
+		that arrive from any arp_ip_target are considered legit.
+
+	arp or 4
+
+		Any arp packet is accepted as a proof that any slave is up,
+		but no IP-based validation is made.
+
+	active_arp or 5
+
+		Validation is performed only for the active slave. Only ARPs
+		that arrive from any arp_ip_target are considered legit. The
+		backup slave validates only arp packets, but doesn't check the
+		source (as if arp_validate=4).
+
+	backup_any or 6
+
+		Validation is performed only for backup slaves. Only ARPs
+		that arrive from any arp_ip_target are considered legit. The
+		active slave validates only arp packets, but doesn't check the
+		source (as if arp_validate=4).
 
 	For the active slave, the validation checks ARP replies to
 	confirm that they were generated by an arp_ip_target.  Since
-- 
1.8.4

^ permalink raw reply related

* [PATCH v2 net-next 05/12] bonding: extend arp_validate to be able to receive unvalidated arp-only traffic
From: Veaceslav Falico @ 2014-01-17 16:58 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389977940-17084-1-git-send-email-vfalico@redhat.com>

Currently we can either receive any traffic as a proff of slave being up,
or only *validated* arp traffic (i.e. with src/dst ip checked).

Add an option to be able to specify if we want to receive non-validated arp
traffic only.

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 |  3 +++
 drivers/net/bonding/bonding.h   | 11 +++++++++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 07ae82d..532a452 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -246,6 +246,9 @@ const struct bond_parm_tbl arp_validate_tbl[] = {
 {	"active",		BOND_ARP_VALIDATE_ACTIVE},
 {	"backup",		BOND_ARP_VALIDATE_BACKUP},
 {	"all",			BOND_ARP_VALIDATE_ALL},
+{	"arp",			BOND_ARP_VALIDATE_ARP},
+{	"active_arp",		BOND_ARP_VALIDATE_ACTIVE_ARP},
+{	"backup_arp",		BOND_ARP_VALIDATE_BACKUP_ARP},
 {	NULL,			-1},
 };
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 955dc48..1fbbf04 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -318,6 +318,11 @@ static inline bool bond_is_active_slave(struct slave *slave)
 #define BOND_ARP_VALIDATE_BACKUP	(1 << BOND_STATE_BACKUP)
 #define BOND_ARP_VALIDATE_ALL		(BOND_ARP_VALIDATE_ACTIVE | \
 					 BOND_ARP_VALIDATE_BACKUP)
+#define BOND_ARP_VALIDATE_ARP		(BOND_ARP_VALIDATE_ALL + 1)
+#define BOND_ARP_VALIDATE_ACTIVE_ARP	(BOND_ARP_VALIDATE_ACTIVE | \
+					 BOND_ARP_VALIDATE_ARP)
+#define BOND_ARP_VALIDATE_BACKUP_ARP	(BOND_ARP_VALIDATE_BACKUP | \
+					 BOND_ARP_VALIDATE_ARP)
 
 static inline int slave_do_arp_validate(struct bonding *bond,
 					struct slave *slave)
@@ -325,6 +330,12 @@ static inline int slave_do_arp_validate(struct bonding *bond,
 	return bond->params.arp_validate & (1 << bond_slave_state(slave));
 }
 
+static inline int slave_do_arp_validate_only(struct bonding *bond,
+					     struct slave *slave)
+{
+	return bond->params.arp_validate & BOND_ARP_VALIDATE_ARP;
+}
+
 /* Get the oldest arp which we've received on this slave for bond's
  * arp_targets.
  */
-- 
1.8.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox