Netdev List
 help / color / mirror / Atom feed
* Convert net_msg_warn, NETDEBUG, & LIMIT_NETDEBUG?
From: Joe Perches @ 2014-11-04 20:46 UTC (permalink / raw)
  To: netdev

net_msg_warn is a sysctl used to control the printk
of a bundle of mostly ipv4/ipv6 logging messages.

Does anyone use it?

NETDEBUG is used 4 times, 2 of which seem senseless
as they are allocation failures messages after an
alloc_skb.  These already get stack dumps.

The other NETDEBUG uses are ESP crypto descriptions.

LIMIT_NETDEBUG is used a lot more.

include/net/sock.h:#define LIMIT_NETDEBUG(fmt, args...) \
include/net/sock.h-     do { if (net_msg_warn && net_ratelimit()) printk(fmt,##args); } while(0)

Most of the LIMIT_NETDEBUG uses are emitted at KERN_DEBUG.

Here is the count of each type of use:
     31 KERN_DEBUG
      2 KERN_ERR
      3 KERN_INFO
     11 KERN_WARNING

Should those KERN_DEBUG uses be converted to
net_dbg_ratelimited so that these uses could be
controlled via dynamic_debug instead of the
net_msg_warn sysctl?

net/dccp/ uses LIMIT_NETDEBUG via DCCP_WARN to
control another 38 KERN_WARNING messages.

The others LIMIT_NETDEBUG uses could be converted
to net_<level>_ratelimited if appropriate.

^ permalink raw reply

* Re: [PATCH v2 net] tcp: zero retrans_stamp if all retrans were acked
From: Marcelo Ricardo Leitner @ 2014-11-04 20:51 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: Netdev, Yuchung Cheng, Eric Dumazet
In-Reply-To: <CADVnQymMioK0USNpaOF2Kb4+zMTDrjJ7r=5Bu51LbirVQPnzyw@mail.gmail.com>

On 04-11-2014 18:10, Neal Cardwell wrote:
> On Tue, Nov 4, 2014 at 2:15 PM, Marcelo Ricardo Leitner
> <mleitner@redhat.com> wrote:
> ...
>> Therefore, now we clear retrans_stamp as soon as all data during the
>> loss window is fully acked.
>>
>> Reported-by: Ueki Kohei
>> Cc: Neal Cardwell <ncardwell@google.com>
>> Cc: Yuchung Cheng <ycheng@google.com>
>> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
>> ---
>>
>> Notes:
>>      v1->v2: fixed compilation issue noticed by Neal
>>
>>   net/ipv4/tcp_input.c | 60 +++++++++++++++++++++++++++-------------------------
>>   1 file changed, 31 insertions(+), 29 deletions(-)
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
> Tested-by: Neal Cardwell <ncardwell@google.com>
>
> Code looks fine, and it passes Yuchung's packetdrill test case for this.
>
> Thanks for finding and fixing this, Marcelo.

And thank you guys for all the assistance on it. Btw, would you send me that 
packetdrill script? I'm curious to see how such corner case could be written 
on it.

Regards,
Marcelo

^ permalink raw reply

* Re: [PATCH v2 5/5] stmmac: pci: remove FSF address
From: David Miller @ 2014-11-04 20:15 UTC (permalink / raw)
  To: xose.vazquez; +Cc: netdev, andriy.shevchenko
In-Reply-To: <54593285.3030201@gmail.com>

From: Xose Vazquez Perez <xose.vazquez@gmail.com>
Date: Tue, 04 Nov 2014 21:09:41 +0100

> David Miller wrote:
> 
>> Andy Shevchenko wrote:
>>> The FSF address is subject to change, thus remove it from the file.
>>> 
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> 
>> I cound 90t instances of this under drivers/net, therefore if this change is
>           ^^^^^^^^^^^^^
>> appropriate I'd rather someone script this and kill it across entire
>> subdirectories.
> 
> It's a bit more, 555:

I meant "nine hundred something", the 't' it my finger slipping down
one row on my keyboard :-)

^ permalink raw reply

* Re: [PATCH 1/1 net-next] igmp: remove camel case definitions
From: David Miller @ 2014-11-04 20:14 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130734-17124-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:52:14 +0100

> use standard uppercase for definitions
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] udp: remove else after return
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130521-16479-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:48:41 +0100

> else is unnecessary after return 0 in __udp4_lib_rcv()
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] inet: frags: remove inline on static in c file
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130244-15003-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:44:04 +0100

> remove __inline__ / inline and let compiler decide what to do
> with static functions
> 
> Inspired-by: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] ipv4: ip_frag_queue function clean-up
From: David Miller @ 2014-11-04 20:12 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415130097-14490-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:41:37 +0100

> -remove unnecessary else after break
> -declare free_it sk_buff * only once (like prev and next)
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

I disagree.

Declaring a local variable in it's inner-most scope is preferable
to moving it to the top level and making the job of the compiler
and the code auditor more difficult.

I'm not applying this, sorry.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] ipv4: remove 0/NULL assignment on static
From: David Miller @ 2014-11-04 20:11 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129866-13852-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:37:46 +0100

> static values are automatically initialized to 0
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] ipv4: use seq_puts instead of seq_printf where possible
From: David Miller @ 2014-11-04 20:11 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129486-13185-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:31:26 +0100

> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] tcp: spelling s/plugable/pluggable
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415129138-11901-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:25:38 +0100

> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] cipso: remove NULL assignment on static
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128759-11264-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:19:19 +0100

> Also add blank line after structure declarations
> 
> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] cipso: include linux/bug.h instead of asm/bug.h
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128541-10891-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:15:41 +0100

> Signed-off-by: Fabian Frederick <fabf@skynet.be>

This is already the case in the net-next tree, therefore this patch is
not applicable.

^ permalink raw reply

* Re: [PATCH v2 net] tcp: zero retrans_stamp if all retrans were acked
From: Neal Cardwell @ 2014-11-04 20:10 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: Netdev, Yuchung Cheng, Eric Dumazet
In-Reply-To: <f58c4b02028e0750e583518608891d20742c0f38.1415128437.git.mleitner@redhat.com>

On Tue, Nov 4, 2014 at 2:15 PM, Marcelo Ricardo Leitner
<mleitner@redhat.com> wrote:
...
> Therefore, now we clear retrans_stamp as soon as all data during the
> loss window is fully acked.
>
> Reported-by: Ueki Kohei
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
> ---
>
> Notes:
>     v1->v2: fixed compilation issue noticed by Neal
>
>  net/ipv4/tcp_input.c | 60 +++++++++++++++++++++++++++-------------------------
>  1 file changed, 31 insertions(+), 29 deletions(-)

Acked-by: Neal Cardwell <ncardwell@google.com>
Tested-by: Neal Cardwell <ncardwell@google.com>

Code looks fine, and it passes Yuchung's packetdrill test case for this.

Thanks for finding and fixing this, Marcelo.

neal

^ permalink raw reply

* Re: [PATCH 1/1 net-next] ipv4: include linux/bug.h instead of asm/bug.h
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128430-10674-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:13:50 +0100

> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH 1/1 net-next] cipso: kerneldoc warning fix
From: David Miller @ 2014-11-04 20:10 UTC (permalink / raw)
  To: fabf; +Cc: linux-kernel, kuznet, jmorris, yoshfuji, kaber, netdev
In-Reply-To: <1415128201-8246-1-git-send-email-fabf@skynet.be>

From: Fabian Frederick <fabf@skynet.be>
Date: Tue,  4 Nov 2014 20:10:01 +0100

> Signed-off-by: Fabian Frederick <fabf@skynet.be>

Applied.

^ permalink raw reply

* Re: [PATCH v2 5/5] stmmac: pci: remove FSF address
From: Xose Vazquez Perez @ 2014-11-04 20:09 UTC (permalink / raw)
  To: netdev, davem, andriy.shevchenko

David Miller wrote:

> Andy Shevchenko wrote:
>> The FSF address is subject to change, thus remove it from the file.
>> 
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> I cound 90t instances of this under drivers/net, therefore if this change is
          ^^^^^^^^^^^^^
> appropriate I'd rather someone script this and kill it across entire
> subdirectories.

It's a bit more, 555:

    134  * 51 Franklin Street, Fifth Floor, Boston, MA 02110, USA
     96  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
     89   51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
     86  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
     44  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA or on the
     43   Boston, MA 02110-1301, USA.
     19  * Temple Place - Suite 330, Boston, MA  02111-1307, USA.
     10   Temple Place - Suite 330, Boston, MA  02111-1307, USA.
      8  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
      5  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
      5  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
      5 # 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
      3  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
      2 # 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA or on the
      1  *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
      1 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110,
      1 # Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
      1  * Boston, MA  02110-1301, USA.
      1  *  Boston, MA 02110-1301, USA.
      1  * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.

^ permalink raw reply

* Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on arndale platform
From: Charles Keepax @ 2014-11-04 20:09 UTC (permalink / raw)
  To: Stam, Michel [FINT]
  Cc: Riku Voipio, davem, linux-usb, netdev, linux-kernel,
	linux-samsung-soc
In-Reply-To: <C89EFD3CD56F64468D3D206D683A8D2203985FA9@ldam-msx2.fugro-nl.local>

On Tue, Nov 04, 2014 at 11:23:06AM +0100, Stam, Michel [FINT] wrote:
> Hello Riku,
> 
> >Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
> >
> >I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
> 
> I don't fully agree here; 
> I would like to point out that this commit is a revert itself. Fixing
> the armdale will then cause breakage in other implementations, such as
> ours. Blankly reverting breaks other peoples' implementations.
> 
> The PHY reset is the thing that breaks ethtool support, so any fix that
> appeases all would have to take existing PHY state into account. 
> 
> I'm not an expert on the ASIX driver, nor the MII, but I think this is
> the cause;
> drivers/net/usb/asix_devices.c:
>    361      asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR,
> BMCR_RESET);
>    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>    363              ADVERTISE_ALL | ADVERTISE_CSMA);
>    364      mii_nway_restart(&dev->mii);
> 
> I would think that the ADVERTISE_ALL is the cause here, as it will reset
> the MII back to default, thus overriding ethtool settings.
> Would an:
> Int reg;
> reg = asix_mdio_read(dev->net,dev->mii.phy_id,MII_ADVERTISE);
> 
> prior to the offending lines, and then;
> 
>    362      asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
>    363             reg);
> 
> solve the problem for you guys?

If I revert the patch in question and add this in:

--- a/drivers/net/usb/asix_devices.c
+++ b/drivers/net/usb/asix_devices.c
@@ -299,6 +299,7 @@ static int ax88772_reset(struct usbnet *dev)
 {
        struct asix_data *data = (struct asix_data *)&dev->data;
        int ret, embd_phy;
+       int reg;
        u16 rx_ctl;

        ret = asix_write_gpio(dev,
@@ -359,8 +360,10 @@ static int ax88772_reset(struct usbnet *dev)
        msleep(150);

        asix_mdio_write(dev->net, dev->mii.phy_id, MII_BMCR, BMCR_RESET);
-       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE,
-                       ADVERTISE_ALL | ADVERTISE_CSMA);
+       reg = asix_mdio_read(dev->net, dev->mii.phy_id, MII_ADVERTISE);
+       if (!reg)
+               reg = ADVERTISE_ALL | ADVERTISE_CSMA;
+       asix_mdio_write(dev->net, dev->mii.phy_id, MII_ADVERTISE, reg);
        mii_nway_restart(&dev->mii);

        ret = asix_write_medium_mode(dev, AX88772_MEDIUM_DEFAULT);

Then things work on Arndale for me. Does that work for you?
Whether that is a sensible fix I don't know however.

> 
> Mind, maybe the read function should take into account the reset value
> of the MII, and set it to ADVERTISE_ALL | ADVERTISE_CSMA. I don't have
> any documention here at the moment.

Yeah I also have no documentation.

Thanks,
Charles

> 
> Is anyone able to confirm my suspicions?
> 
> Kind regards,
> 
> Michel Stam
> -----Original Message-----
> From: Riku Voipio [mailto:riku.voipio@iki.fi] 
> Sent: Tuesday, November 04, 2014 10:44 AM
> To: Stam, Michel [FINT]
> Cc: Riku Voipio; davem@davemloft.net; linux-usb@vger.kernel.org;
> netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-samsung-soc@vger.kernel.org; ckeepax@opensource.wolfsonmicro.com
> Subject: Re: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net
> on arndale platform
> 
> On Tue, Nov 04, 2014 at 09:19:26AM +0100, Stam, Michel [FINT] wrote:
> > Interesting, as the commit itself is a revert from a kernel back to 
> > 2.6 somewhere. The problem I had is related to the PHY being reset on 
> > interface-up, can you confirm that you require this?
> 
> I can't confirm what exactly is needed on arndale. I'm neither expert in
> USB or ethernet. However, I can confirm that without the PHY reset,
> networking doesn't work on arndale.
> 
> I now see someone else has the same problem, adding Charles to CC.
> 
> http://www.spinics.net/lists/linux-usb/msg116656.html
> 
> > Reverting this
> > breaks ethtool support in turn.
> 
> Fixing a bug (ethtool support) must not cause breakage elsewhere (in
> this case on arndale). This is now a regression of functionality from
> 3.17.
> 
> I think it would better to revert the change now and with less hurry
> introduce a ethtool fix that doesn't break arndale.
>  
> > Kind regards,
> > 
> > Michel Stam
> > 
> > -----Original Message-----
> > From: Riku Voipio [mailto:riku.voipio@iki.fi]
> > Sent: Tuesday, November 04, 2014 8:23 AM
> > To: davem@davemloft.net; Stam, Michel [FINT]
> > Cc: linux-usb@vger.kernel.org; netdev@vger.kernel.org; 
> > linux-kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org
> > Subject: "asix: Don't reset PHY on if_up for ASIX 88772" breaks net on
> 
> > arndale platform
> > 
> > Hi,
> > 
> > With 3.18-rc3, asix on arndale (samsung exynos 5250 based board), 
> > fails to work. Interface is initialized but network traffic seem not 
> > to pass through. With kernel IP config the result looks like:
> > 
> > [    3.323275] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [    3.419151] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [    3.424735] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [    3.432196] usb 3-3.2.4: Product: AX88772 
> > [    3.436279] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [    3.441486] usb 3-3.2.4: SerialNumber: 000001
> > [    3.447530] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized):
> > invalid hw address, using random
> > [    3.764352] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> de:a2:66:bf:ca:4f
> > [    4.488773] asix 3-3.2.4:1.0 eth0: link down
> > [    5.690025] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [    5.712947] Sending DHCP requests ...... timed out!
> > [   83.165303] IP-Config: Retrying forever (NFS root)...
> > [   83.170397] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [   83.192944] Sending DHCP requests .....
> > 
> > Similar results also with dhclient. Git bisect identified the breaking
> 
> > commit as:
> > 
> > commit 3cc81d85ee01e5a0b7ea2f4190e2ed1165f53c31
> > Author: Michel Stam <m.stam@fugro.nl>
> > Date:   Thu Oct 2 10:22:02 2014 +0200
> > 
> >     asix: Don't reset PHY on if_up for ASIX 88772
> > 
> > Taking 3.18-rc3 and that commit reverted, network works again:
> > 
> > [    3.303500] usb 3-3.2.4: new high-speed USB device number 4 using
> > exynos-ehci
> > [    3.399375] usb 3-3.2.4: New USB device found, idVendor=0b95,
> > idProduct=772a
> > [    3.404963] usb 3-3.2.4: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [    3.412424] usb 3-3.2.4: Product: AX88772 
> > [    3.416508] usb 3-3.2.4: Manufacturer: ASIX Elec. Corp.
> > [    3.421715] usb 3-3.2.4: SerialNumber: 000001
> > [    3.427755] asix 3-3.2.4:1.0 (unnamed net_device) (uninitialized):
> > invalid hw address, using random
> > [    3.744837] asix 3-3.2.4:1.0 eth0: register 'asix' at
> > usb-12110000.usb-3.2.4, ASIX AX88772 USB 2.0 Ethernet,
> 12:59:f1:a8:43:90
> > [    7.098998] asix 3-3.2.4:1.0 eth0: link up, 100Mbps, full-duplex,
> lpa
> > 0xC5E1
> > [    7.118258] Sending DHCP requests ., OK
> > [    9.753259] IP-Config: Got DHCP answer from 192.168.1.1, my address
> > is 192.168.1.111
> > 
> > There might something wrong on the samsung platform code (I understand
> 
> > the USB on arndale is "funny"), but this is still an regression from 
> > 3.17.
> > 
> > Riku

^ permalink raw reply

* Re: [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: David Miller @ 2014-11-04 20:07 UTC (permalink / raw)
  To: joe
  Cc: fabf, linux-kernel, steffen.klassert, herbert, kuznet, jmorris,
	yoshfuji, kaber, netdev
In-Reply-To: <1415129631.23168.3.camel@perches.com>

From: Joe Perches <joe@perches.com>
Date: Tue, 04 Nov 2014 11:33:51 -0800

> On Tue, 2014-11-04 at 20:28 +0100, Fabian Frederick wrote:
> 
> trivia:
> 
>> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> []
>> @@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>>  	if (elen <= 0)
>>  		goto out;
>>  
>> -	if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
>> +	err = skb_cow_data(skb, 0, &trailer);
>> +
>> +	if (err < 0)
>>  		goto out;
> 
> Generally it's better/more common to use
> 
> 	foo = bar();
> 	if (!foo)
> 		[error_handler...]
> 	
> without the blank line between the set
> and the test.

+1

^ permalink raw reply

* Re: [PATCH 00/13] net_sched: misc cleanups and improvements
From: David Miller @ 2014-11-04 19:56 UTC (permalink / raw)
  To: eric.dumazet; +Cc: xiyou.wangcong, netdev, jhs
In-Reply-To: <1415125250.25370.2.camel@edumazet-glaptop2.roam.corp.google.com>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 04 Nov 2014 10:20:50 -0800

> On Tue, 2014-11-04 at 09:56 -0800, Cong Wang wrote:
>> Minor code cleanups for TC qdiscs and filters, each patch has
>> more details.
>> 
>> Cong Wang (13):
>>       net_sched: refactor out tcf_exts
>>       net_sched: introduce qdisc_peek() helper function
>>       net_sched: rename ->gso_skb to ->dequeued_skb
>>       net_sched: rename qdisc_drop() to qdisc_drop_skb()
>>       net_sched: introduce qdisc_drop() helper function
>>       net_sched: move some qdisc flag into qdisc ops
>>       net_sched: move TCQ_F_MQROOT into qdisc ops
>>       net_sched: use a flag to indicate fifo qdiscs instead of the name
>>       net_sched: redefine qdisc_create_dflt()
>>       net_sched: forbid setting default qdisc to inappropriate ones
>>       net_sched: remove hashmask from Qdisc_class_hash
>>       net_sched: remove useless qdisc_stab_lock
>>       net_sched: return NULL instead of ERR_PTR for qdisc_alloc()
>> 
> 
> NACK for the whole serie.
> 
> I am tired of your inexistent changelogs and code churn in qdisc.
> 
> If you do not care of writing good changelogs and explain why you want
> all this, why should I care spending time to review all this stuff ?
> 
> What is the plan, beyond all this code churn ?

+1

^ permalink raw reply

* Re: [PATCH 02/20] selftests/net: move test out of Makefile into a shell script
From: David Miller @ 2014-11-04 19:53 UTC (permalink / raw)
  To: shuahkh
  Cc: gregkh, akpm, mmarek, keescook, tranmanphong, dh.herrmann, hughd,
	bobby.prani, ebiederm, serge.hallyn, linux-kbuild, linux-kernel,
	linux-api, netdev
In-Reply-To: <51d57725e89809806bd8af5555af341296cc4dd7.1415117102.git.shuahkh@osg.samsung.com>

From: Shuah Khan <shuahkh@osg.samsung.com>
Date: Tue,  4 Nov 2014 10:10:58 -0700

> Currently bpf test run from the Makefile. Move it out of the
> Makefile to be run from a shell script to allow the test to
> be run as stand-alone test, in addition to allowing the test
> run from a make target.
> 
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>

Acked-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* [PATCH 1/1 net-next] igmp: remove camel case definitions
From: Fabian Frederick @ 2014-11-04 19:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev

use standard uppercase for definitions

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv4/igmp.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index fb70e3e..3f80513 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -112,17 +112,17 @@
 #ifdef CONFIG_IP_MULTICAST
 /* Parameter names and values are taken from igmp-v2-06 draft */
 
-#define IGMP_V1_Router_Present_Timeout		(400*HZ)
-#define IGMP_V2_Router_Present_Timeout		(400*HZ)
-#define IGMP_V2_Unsolicited_Report_Interval	(10*HZ)
-#define IGMP_V3_Unsolicited_Report_Interval	(1*HZ)
-#define IGMP_Query_Response_Interval		(10*HZ)
-#define IGMP_Query_Robustness_Variable		2
+#define IGMP_V1_ROUTER_PRESENT_TIMEOUT		(400*HZ)
+#define IGMP_V2_ROUTER_PRESENT_TIMEOUT		(400*HZ)
+#define IGMP_V2_UNSOLICITED_REPORT_INTERVAL	(10*HZ)
+#define IGMP_V3_UNSOLICITED_REPORT_INTERVAL	(1*HZ)
+#define IGMP_QUERY_RESPONSE_INTERVAL		(10*HZ)
+#define IGMP_QUERY_ROBUSTNESS_VARIABLE		2
 
 
-#define IGMP_Initial_Report_Delay		(1)
+#define IGMP_INITIAL_REPORT_DELAY		(1)
 
-/* IGMP_Initial_Report_Delay is not from IGMP specs!
+/* IGMP_INITIAL_REPORT_DELAY is not from IGMP specs!
  * IGMP specs require to report membership immediately after
  * joining a group, but we delay the first report by a
  * small interval. It seems more natural and still does not
@@ -879,15 +879,15 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
 		if (ih->code == 0) {
 			/* Alas, old v1 router presents here. */
 
-			max_delay = IGMP_Query_Response_Interval;
+			max_delay = IGMP_QUERY_RESPONSE_INTERVAL;
 			in_dev->mr_v1_seen = jiffies +
-				IGMP_V1_Router_Present_Timeout;
+				IGMP_V1_ROUTER_PRESENT_TIMEOUT;
 			group = 0;
 		} else {
 			/* v2 router present */
 			max_delay = ih->code*(HZ/IGMP_TIMER_SCALE);
 			in_dev->mr_v2_seen = jiffies +
-				IGMP_V2_Router_Present_Timeout;
+				IGMP_V2_ROUTER_PRESENT_TIMEOUT;
 		}
 		/* cancel the interface change timer */
 		in_dev->mr_ifc_count = 0;
@@ -899,7 +899,7 @@ static bool igmp_heard_query(struct in_device *in_dev, struct sk_buff *skb,
 		return true;	/* ignore bogus packet; freed by caller */
 	} else if (IGMP_V1_SEEN(in_dev)) {
 		/* This is a v3 query with v1 queriers present */
-		max_delay = IGMP_Query_Response_Interval;
+		max_delay = IGMP_QUERY_RESPONSE_INTERVAL;
 		group = 0;
 	} else if (IGMP_V2_SEEN(in_dev)) {
 		/* this is a v3 query with v2 queriers present;
@@ -1218,7 +1218,7 @@ static void igmp_group_added(struct ip_mc_list *im)
 		return;
 	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev)) {
 		spin_lock_bh(&im->lock);
-		igmp_start_timer(im, IGMP_Initial_Report_Delay);
+		igmp_start_timer(im, IGMP_INITIAL_REPORT_DELAY);
 		spin_unlock_bh(&im->lock);
 		return;
 	}
@@ -1541,7 +1541,7 @@ static struct in_device *ip_mc_find_dev(struct net *net, struct ip_mreqn *imr)
 int sysctl_igmp_max_memberships __read_mostly = IP_MAX_MEMBERSHIPS;
 int sysctl_igmp_max_msf __read_mostly = IP_MAX_MSF;
 #ifdef CONFIG_IP_MULTICAST
-int sysctl_igmp_qrv __read_mostly = IGMP_Query_Robustness_Variable;
+int sysctl_igmp_qrv __read_mostly = IGMP_QUERY_ROBUSTNESS_VARIABLE;
 #endif
 
 static int ip_mc_del1_src(struct ip_mc_list *pmc, int sfmode,
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] udp: remove else after return
From: Fabian Frederick @ 2014-11-04 19:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev

else is unnecessary after return 0 in __udp4_lib_rcv()

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv4/udp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index cd0db54..cf0cece 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1777,14 +1777,14 @@ int __udp4_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 		if (ret > 0)
 			return -ret;
 		return 0;
-	} else {
-		if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
-			return __udp4_lib_mcast_deliver(net, skb, uh,
-					saddr, daddr, udptable);
-
-		sk = __udp4_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
 	}
 
+	if (rt->rt_flags & (RTCF_BROADCAST|RTCF_MULTICAST))
+		return __udp4_lib_mcast_deliver(net, skb, uh,
+				saddr, daddr, udptable);
+
+	sk = __udp4_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
+
 	if (sk != NULL) {
 		int ret;
 
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1 net-next] inet: frags: remove inline on static in c file
From: Fabian Frederick @ 2014-11-04 19:44 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev

remove __inline__ / inline and let compiler decide what to do
with static functions

Inspired-by: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv4/ip_fragment.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2811cc1..4d964da 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -80,7 +80,7 @@ struct ipq {
 	struct inet_peer *peer;
 };
 
-static inline u8 ip4_frag_ecn(u8 tos)
+static u8 ip4_frag_ecn(u8 tos)
 {
 	return 1 << (tos & INET_ECN_MASK);
 }
@@ -148,7 +148,7 @@ static void ip4_frag_init(struct inet_frag_queue *q, const void *a)
 		inet_getpeer_v4(net->ipv4.peers, arg->iph->saddr, 1) : NULL;
 }
 
-static __inline__ void ip4_frag_free(struct inet_frag_queue *q)
+static void ip4_frag_free(struct inet_frag_queue *q)
 {
 	struct ipq *qp;
 
@@ -160,7 +160,7 @@ static __inline__ void ip4_frag_free(struct inet_frag_queue *q)
 
 /* Destruction primitives. */
 
-static __inline__ void ipq_put(struct ipq *ipq)
+static void ipq_put(struct ipq *ipq)
 {
 	inet_frag_put(&ipq->q, &ip4_frags);
 }
@@ -236,7 +236,7 @@ out:
 /* Find the correct entry in the "incomplete datagrams" queue for
  * this IP datagram, and create new one, if nothing is found.
  */
-static inline struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
+static struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
 {
 	struct inet_frag_queue *q;
 	struct ip4_create_arg arg;
@@ -256,7 +256,7 @@ static inline struct ipq *ip_find(struct net *net, struct iphdr *iph, u32 user)
 }
 
 /* Is the fragment too far ahead to be part of ipq? */
-static inline int ip_frag_too_far(struct ipq *qp)
+static int ip_frag_too_far(struct ipq *qp)
 {
 	struct inet_peer *peer = qp->peer;
 	unsigned int max = sysctl_ipfrag_max_dist;
@@ -795,16 +795,16 @@ static void __init ip4_frags_ctl_register(void)
 	register_net_sysctl(&init_net, "net/ipv4", ip4_frags_ctl_table);
 }
 #else
-static inline int ip4_frags_ns_ctl_register(struct net *net)
+static int ip4_frags_ns_ctl_register(struct net *net)
 {
 	return 0;
 }
 
-static inline void ip4_frags_ns_ctl_unregister(struct net *net)
+static void ip4_frags_ns_ctl_unregister(struct net *net)
 {
 }
 
-static inline void __init ip4_frags_ctl_register(void)
+static void __init ip4_frags_ctl_register(void)
 {
 }
 #endif
-- 
1.9.3

^ permalink raw reply related

* Re: [PATCH 1/1 net-next] esp4: remove assignment in if condition
From: Daniel Borkmann @ 2014-11-04 19:42 UTC (permalink / raw)
  To: Fabian Frederick
  Cc: linux-kernel, Steffen Klassert, Herbert Xu, David S. Miller,
	Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
	Patrick McHardy, netdev
In-Reply-To: <1415129311-12480-1-git-send-email-fabf@skynet.be>

On 11/04/2014 08:28 PM, Fabian Frederick wrote:
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
>   net/ipv4/esp4.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
> index 360b565..9dd66ee 100644
> --- a/net/ipv4/esp4.c
> +++ b/net/ipv4/esp4.c
> @@ -392,8 +392,11 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
>   	if (elen <= 0)
>   		goto out;
>
> -	if ((err = skb_cow_data(skb, 0, &trailer)) < 0)
> +	err = skb_cow_data(skb, 0, &trailer);
> +

If you already feel the need to change this (?), then please don't
add an extra newline here ...

> +	if (err < 0)
>   		goto out;
> +
>   	nfrags = err;
>
>   	assoclen = sizeof(*esph);
>

^ permalink raw reply

* [PATCH 1/1 net-next] ipv4: ip_frag_queue function clean-up
From: Fabian Frederick @ 2014-11-04 19:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: Fabian Frederick, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev

-remove unnecessary else after break
-declare free_it sk_buff * only once (like prev and next)

Signed-off-by: Fabian Frederick <fabf@skynet.be>
---
 net/ipv4/ip_fragment.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 2811cc1..153c402 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -317,7 +317,7 @@ static int ip_frag_reinit(struct ipq *qp)
 /* Add new segment to existing queue. */
 static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
 {
-	struct sk_buff *prev, *next;
+	struct sk_buff *prev, *next, *free_it;
 	struct net_device *dev;
 	int flags, offset;
 	int ihl, end;
@@ -432,23 +432,23 @@ found:
 			if (next->ip_summed != CHECKSUM_UNNECESSARY)
 				next->ip_summed = CHECKSUM_NONE;
 			break;
-		} else {
-			struct sk_buff *free_it = next;
+		}
 
-			/* Old fragment is completely overridden with
-			 * new one drop it.
-			 */
-			next = next->next;
+		free_it = next;
 
-			if (prev)
-				prev->next = next;
-			else
-				qp->q.fragments = next;
+		/* Old fragment is completely overridden with
+		 * new one drop it.
+		 */
+		next = next->next;
 
-			qp->q.meat -= free_it->len;
-			sub_frag_mem_limit(&qp->q, free_it->truesize);
-			kfree_skb(free_it);
-		}
+		if (prev)
+			prev->next = next;
+		else
+			qp->q.fragments = next;
+
+		qp->q.meat -= free_it->len;
+		sub_frag_mem_limit(&qp->q, free_it->truesize);
+		kfree_skb(free_it);
 	}
 
 	FRAG_CB(skb)->offset = offset;
-- 
1.9.3

^ 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