Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 2/2] net/can: add module alias to can protocol drivers
From: David Miller @ 2009-07-15 18:21 UTC (permalink / raw)
  To: LW; +Cc: netdev, oliver, herbert, wg, urs.thuermann, urs
In-Reply-To: <19037.40313.827769.602243@ipc1.ka-ro>

From: Lothar Waßmann <LW@KARO-electronics.de>
Date: Wed, 15 Jul 2009 11:12:25 +0200

> Add appropriate MODULE_ALIAS() to facilitate autoloading of can protocol drivers
> 
> Signed-off-by: Lothar Wassmann <LW@KARO-electronics.de>
> Acked-by: Oliver Hartkopp <oliver@hartkopp.net>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] net/can bugfix: use after free bug in can protocol drivers
From: David Miller @ 2009-07-15 18:20 UTC (permalink / raw)
  To: LW; +Cc: wg, oliver, herbert, netdev, urs.thuermann, urs
In-Reply-To: <19037.40189.520170.8242@ipc1.ka-ro>

From: Lothar Waßmann <LW@KARO-electronics.de>
Date: Wed, 15 Jul 2009 11:10:21 +0200

> Fix a use after free bug in can protocol drivers
> 
> The release functions of the can protocol drivers lack a call to
> sock_orphan() which leads to referencing freed memory under certain
> circumstances.
> 
> This patch fixes a bug reported here:
> https://lists.berlios.de/pipermail/socketcan-users/2009-July/000985.html
> 
> Signed-off-by: Lothar Wassmann <LW@KARO-electronics.de>
> Acked-by: Oliver Hartkopp <oliver@hartkopp.net>

Applied.

^ permalink raw reply

* Re: [PATCH] iproute2 flush: handle larger tables and deleted entries
From: Gautam Kachroo @ 2009-07-15 17:50 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev
In-Reply-To: <4A5DF369.1090107@trash.net>

On Wed, Jul 15, 2009 at 8:19 AM, Patrick McHardy<kaber@trash.net> wrote:
> Gautam Kachroo wrote:
>> On Tue, Jul 14, 2009 at 2:38 AM, Patrick McHardy<kaber@trash.net> wrote:
>>> Gautam Kachroo wrote:
>>>> use a new netlink socket when sending flush messages to avoid reading
>>>> any pending data on the existing netlink socket.
>>>>
>>>> read all of the response from the netlink request -- this response can
>>>> be split over multiple recv calls, pretty much one per netlink request
>>>> message. ENOENT errors, which correspond to attempts to delete an
>>>> already deleted entry, are ignored. Other errors are not ignored.
>>>
>>> In which case would there be any pending data? From what I can see,
>>> this can only happen when using batching, but in that case the
>>> previous command should continue reading until it has received all
>>> responses (which the netlink functions appear to be doing properly).
>>
>> What is the "previous command"?
>
> The last command before the one executing when using batching.

This is independent of batching (I assume you're referring to the
-batch option to the ip command).
It happens when running a command like "ip neigh flush to 0.0.0.0/0"
if there are many neighbor entries.

The implementation of flush commands, e.g. ip neigh flush, sends a
dump request, e.g. RTM_GETNEIGH, and then sends requests, e.g.
RTM_DELNEIGH, *while* there can be unread data from the dump request.
There would be unread data if the response to the dump request was
split over multiple calls to recvmsg.

>> Are you referring to rtnl_dump_filter? If rtnl_send_check comes across
>> a failure, rtnl_dump_filter will not continue reading.
>>
>> Here's the situation that I'm referring to:
>>
>> If rtnl_send_check detects an error, it returns -1. rtnl_send_check is
>> called from flush_update. The multiple implementations of flush_update
>> (e.g. in ipneigh.c, ipaddress.c) propagate this return value to their
>> caller, e.g. print_neigh or print_addrinfo.
>>
>> print_neigh, print_addrinfo, etc. are called from rtnl_dump_filter.
>> rtnl_dump_filter sits in a loop calling recvmsg on the netlink socket.
>> However, it returns the error value if the filter function (e.g.
>> print_neigh) returns an error. In this case, rtnl_dump_filter can
>> return before it's read all the responses.
>> The error return from rtnl_dump_filter causes the program to exit.
>
> Yes, and I agree with your patch so far. My question is why you
> need another socket.
>
>> use a new netlink socket when sending flush messages to avoid reading
>> any pending data on the existing netlink socket.
>
> Under what circumstances would there be pending data when
> performing a new iproute operation?

As above, it's not that there is pending data when performing a new
iproute operation, it's that there can be pending data while
performing a single iproute operation, namely ip <object> flush.
The benefit of a new socket is that it won't have any data from the
dump request waiting for it.

thanks,
-gk

^ permalink raw reply

* Re: [PATCH 2/2] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Grant Likely @ 2009-07-15 17:18 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: linuxppc-dev, Kumar Gala, netdev
In-Reply-To: <1247671133-12148-2-git-send-email-wd@denx.de>

On Wed, Jul 15, 2009 at 9:18 AM, Wolfgang Denk<wd@denx.de> wrote:
> This patch adds error checking and prevents clobbering unrelated bits
> (reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
> register on MPC52xx systems.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>

As I mentioned in the other patch, I don't want the 5121 and 5200 FEC
devices using common code for this.  It is a tiny block of code and
they are different devices.  Just open code the needed calculation
into this driver.

g.

> ---
>  drivers/net/fec_mpc52xx.c     |    2 +-
>  drivers/net/fec_mpc52xx_phy.c |    6 ++++--
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cc78633..b69d440 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
>        /* set phy speed.
>         * this can't be done in phy driver, since it needs to be called
>         * before fec stuff (even on resume) */
> -       out_be32(&fec->mii_speed, priv->mdio_speed);
> +       clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
>  }
>
>  /**
> diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
> index 31e6d62..f733d43 100644
> --- a/drivers/net/fec_mpc52xx_phy.c
> +++ b/drivers/net/fec_mpc52xx_phy.c
> @@ -105,8 +105,10 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
>        dev_set_drvdata(dev, bus);
>
>        /* set MII speed */
> -       out_be32(&priv->regs->mii_speed,
> -               ((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
> +       i = mpc5xxx_get_mii_speed(of);
> +       if (i<0)
> +               goto out_unmap;
> +       clrsetbits_be32(&priv->regs->mii_speed, 0x7E, i);
>
>        err = of_mdiobus_register(bus, np);
>        if (err)
> --
> 1.6.0.6
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: why some packets lost during transmission?
From: Jesper Dangaard Brouer @ 2009-07-15 17:17 UTC (permalink / raw)
  To: jon_zhou; +Cc: netdev
In-Reply-To: <60230E21F17E384C9BF0B9C98D9E3FDC04B4D3D8@sgp-sg-mb02.sgp.agilent.com>



On Wed, 15 Jul 2009, jon_zhou@agilent.com wrote:

> I try to send packets like this:
>
> for (i=0;i<loop;i++)
> {
>    dev->hard_start_xmit(my_skb,dev);
> }
>
> the 'dev' point to network device.
>
> but too many packets lost if the 'loop' is big(i.e. 5000), I suspect the 
> tx buffer of the NIC is full,but the driver does not print any related 
> info.

Hint, look at the return value... it returns NETDEV_TX_OK if its okay.

> how does the upper layer(i.e.arp,qdisc) handle this situation?


Have you looked at pktgen?

Doc:
   Documentation/networking/pktgen.txt

Code:
   net/core/pktgen.c

Look at function pktgen_xmit() and notice that in the latest kernels 
the dev->hard_start_xmit function pointer has been changed to 
dev->netdev_ops->ndo_start_xmit.


Cheers,
   Jesper Brouer

--
-------------------------------------------------------------------
MSc. Master of Computer Science
Dept. of Computer Science, University of Copenhagen
Author of http://www.adsl-optimizer.dk
-------------------------------------------------------------------

^ permalink raw reply

* Re: [PATCH 1/2 v3] fs_enet/mii-fec.c: fix MII speed calculation
From: Grant Likely @ 2009-07-15 17:17 UTC (permalink / raw)
  To: Wolfgang Denk; +Cc: linuxppc-dev, netdev
In-Reply-To: <1247671133-12148-1-git-send-email-wd@denx.de>

On Wed, Jul 15, 2009 at 9:18 AM, Wolfgang Denk<wd@denx.de> wrote:
> The MII speed calculation was based on the CPU clock (ppc_proc_freq),
> but for MPC512x we must use the bus clock instead.
>
> This patch makes it use the correct clock and makes sure we don't
> clobber reserved bits in the MII_SPEED register.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>
> ---
> Please ignore patch v2, it's crap.
> Hope this is a bit better.
>
>  arch/powerpc/include/asm/mpc5xxx.h   |   10 +++++++++
>  arch/powerpc/sysdev/mpc5xxx_clocks.c |   37 ++++++++++++++++++++++++++++++++++

Drop the common code bit.  The 5200 and 5121 are different devices and
it is a tiny bit of code.  I don't think there is any benefit to
having it as a common function.  Just roll the get_mii_speed function
in the mii-fec driver itself.

Also, this patch can be quite a bit simpler if you use the .data
pointer in the drivers match table to specify the function used to
return the bus clock speed.  Something like this:

static struct of_device_id fs_enet_mdio_fec_match[] = {
	{
		.compatible = "fsl,pq1-fec-mdio",
	},
#if defined(CONFIG_PPC_MPC512x)
	{
		.compatible = "fsl,mpc5121-fec-mdio",
		.data = mpc5xxx_get_bus_frequency,
	},
#endif
	{},
};

and

int *get_bus_freq(of_node *) = data;
if (get_bus_freq)
        bus_freq = get_bus_freq(np);
else
        bus_freq = ppc_proc_freq / 2;

... then do the regular calculation here and add in the additional
robustification you did in this patch.

Heck, you could even eliminate the if/else above if the normal case
you have a get_bus_speed function for the original ppc_proc_freq case,
but I'm not sure if it is worth it.


>  drivers/net/fs_enet/mii-fec.c        |   13 +++++++++--
>  3 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/mpc5xxx.h b/arch/powerpc/include/asm/mpc5xxx.h
> index 5ce9c5f..86ab29f 100644
> --- a/arch/powerpc/include/asm/mpc5xxx.h
> +++ b/arch/powerpc/include/asm/mpc5xxx.h
> @@ -15,8 +15,18 @@
>
>  #ifndef __ASM_POWERPC_MPC5xxx_H__
>  #define __ASM_POWERPC_MPC5xxx_H__
> +#include <linux/of_platform.h>
>
>  extern unsigned long mpc5xxx_get_bus_frequency(struct device_node *node);
>
> +#if defined(CONFIG_PPC_MPC512x) || defined(CONFIG_PPC_MPC52xx)
> +extern int mpc5xxx_get_mii_speed(struct of_device *ofdev);
> +#else
> +static inline int mpc5xxx_get_mii_speed(struct of_device *ofdev)
> +{
> +       return -1;
> +}
> +#endif
> +
>  #endif /* __ASM_POWERPC_MPC5xxx_H__ */
>
> diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> index 34e12f9..e26d12b 100644
> --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c
> +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c
> @@ -31,3 +31,40 @@ mpc5xxx_get_bus_frequency(struct device_node *node)
>        return p_bus_freq ? *p_bus_freq : 0;
>  }
>  EXPORT_SYMBOL(mpc5xxx_get_bus_frequency);
> +
> +/**
> + *     mpc5xxx_get_mii_speed - Get the MII_SPEED value
> + *     @node:  device node
> + *
> + *     Returns the MII_SPEED value for MPC512x and MPC52xx systems.
> + *     The value gets computed such that the resulting MDC frequency
> + *     is 2.5 MHz or lower.
> + */
> +
> +int
> +mpc5xxx_get_mii_speed(struct of_device *ofdev)
> +{
> +       unsigned int clock, speed;
> +
> +       clock = mpc5xxx_get_bus_frequency(ofdev->node);
> +
> +       if (!clock) {
> +               dev_err(&ofdev->dev, "could not determine IPS/IPB clock\n");
> +               return -ENODEV;
> +       }
> +
> +       /* scale for a MII clock <= 2.5 MHz */
> +       speed = (clock + 2499999) / 2500000;
> +
> +       /* only 6 bits available for MII speed */
> +       if (speed > 0x3F) {
> +               speed = 0x3F;
> +               dev_err(&ofdev->dev,
> +                       "MII clock (%d MHz) exceeds max (2.5 MHz)\n",
> +                       clock / speed);
> +       }
> +
> +       /* Field is in bits 25:30 of MII_SPEED register */
> +       return speed << 1;
> +}
> +EXPORT_SYMBOL(mpc5xxx_get_mii_speed);
> diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.c
> index 75a0999..a28d39f 100644
> --- a/drivers/net/fs_enet/mii-fec.c
> +++ b/drivers/net/fs_enet/mii-fec.c
> @@ -36,6 +36,7 @@
>  #include <asm/pgtable.h>
>  #include <asm/irq.h>
>  #include <asm/uaccess.h>
> +#include <asm/mpc5xxx.h>
>
>  #include "fs_enet.h"
>  #include "fec.h"
> @@ -103,7 +104,6 @@ static int fs_enet_fec_mii_reset(struct mii_bus *bus)
>  static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
>                                         const struct of_device_id *match)
>  {
> -       struct device_node *np = NULL;
>        struct resource res;
>        struct mii_bus *new_bus;
>        struct fec_info *fec;
> @@ -133,13 +133,20 @@ static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
>        if (!fec->fecp)
>                goto out_fec;
>
> -       fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
> +       if (of_device_is_compatible(ofdev->node, "fsl,mpc5121-fec-mdio")) {
> +               i = mpc5xxx_get_mii_speed(ofdev);
> +               if (i < 0)
> +                       goto out_unmap_regs;
> +               fec->mii_speed = i;
> +       } else {
> +               fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
> +       }
>
>        setbits32(&fec->fecp->fec_r_cntrl, FEC_RCNTRL_MII_MODE);
>        setbits32(&fec->fecp->fec_ecntrl, FEC_ECNTRL_PINMUX |
>                                          FEC_ECNTRL_ETHER_EN);
>        out_be32(&fec->fecp->fec_ievent, FEC_ENET_MII);
> -       out_be32(&fec->fecp->fec_mii_speed, fec->mii_speed);
> +       clrsetbits_be32(&fec->fecp->fec_mii_speed, 0x7E, fec->mii_speed);
>
>        new_bus->phy_mask = ~0;
>        new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> --
> 1.6.0.6
>
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Using splice to carry Xen granted pages through the network stack?
From: Jeremy Fitzhardinge @ 2009-07-15 17:14 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Andrew Warfield, Xen-devel, Jake Wires, Herbert Xu, NetDev,
	Rusty Russell, Linux Kernel Mailing List, Chris Mason,
	Daniel Stodden

[ I originally sent this in March or so, but I don't think you
responded, and this still seems like an interesting idea. ]

Hi Jens,

In a thread from December last year ("Support for zero-copy TCP transmit
of user space data") you suggested using the splice machinery as a way
to implement zero-copy transmit for iscsi data.

I have a similar problem relating to doing zero-copy transmit of pages
granted from another Xen domain.  These are pages which are in some ways
similar to device mappings (they have no struct page unless we jump
through some hoops to give them one, for example) which are given to us
by another domain (virtual machine)'s network stack since they contain
data they want to transmit through our stack.  Once the stack has
finished with the pages we need to get them back to return to the
original domain (and definitely not let them get freed into the normal
kernel pool).

I've been looking a little bit at the splice stuff to work out how it
might be useful in this case.  One issue struct page; I guess we really
can't get away from having one for granted pages and still get the full
value of using splice (especially I can see also see it being useful on
the block side of the world as well).  So, we can jump through hoops for
that.

But the general plumbing of splice into the network stack seems to have
been an open issue since you introduced it 3 years ago (using as a
reference http://lwn.net/Articles/181169/).  Has any work been done on
this aspect since?  What would need to be done to do it?  Would it mean
wiring up a pipe_buffer_operations to operate with something like
Rusty's skb_shared_info destructor?

Thanks,
   J

^ permalink raw reply

* Achieved 10Gbit/s bidirectional routing
From: Jesper Dangaard Brouer @ 2009-07-15 16:50 UTC (permalink / raw)
  To: netdev@vger.kernel.org
  Cc: David S. Miller, Robert Olsson, Waskiewicz Jr, Peter P,
	Ronciak, John, jesse.brandeburg, Stephen Hemminger,
	Linux Kernel Mailing List


I'm giving a talk at LinuxCon, about 10Gbit/s routing on standard
hardware running Linux.

  http://linuxcon.linuxfoundation.org/meetings/1585
  https://events.linuxfoundation.org/lc09o17

I'm getting some really good 10Gbit/s bidirectional routing results
with Intels latest 82599 chip. (I got two pre-release engineering
samples directly from Intel, thanks Peter)

Using a Core i7-920, and tuning the memory according to the RAMs
X.M.P. settings DDR3-1600MHz, notice this also increases the QPI to
6.4GT/s.  (Motherboard P6T6 WS revolution)

With big 1514 bytes packets, I can basically do 10Gbit/s wirespeed
bidirectional routing.

Notice bidirectional routing means that we actually has to move approx
40Gbit/s through memory and in-and-out of the interfaces.

Formatted quick view using 'ifstat -b'

  eth31-in   eth31-out   eth32-in  eth32-out
    9.57  +    9.52  +     9.51 +     9.60  = 38.20 Gbit/s
    9.60  +    9.55  +     9.52 +     9.62  = 38.29 Gbit/s
    9.61  +    9.53  +     9.52 +     9.62  = 38.28 Gbit/s
    9.61  +    9.53  +     9.54 +     9.62  = 38.30 Gbit/s

[Adding an extra NIC]

Another observation is that I'm hitting some kind of bottleneck on the
PCI-express switch.  Adding an extra NIC in a PCIe slot connected to
the same PCIe switch, does not scale beyond 40Gbit/s collective
throughput.

But, I happened to have a special motherboard ASUS P6T6 WS revolution,
which has an additional PCIe switch chip NVIDIA's NF200.

Connecting two dual port 10GbE NICs via two different PCI-express
switch chips, makes things scale again!  I have achieved a collective
throughput of 66.25 Gbit/s.  This results is also influenced by my
pktgen machines cannot keep up, and I'm getting closer to the memory
bandwidth limits.

FYI: I found a really good reference explaining the PCI-express
architecture, written by Intel:

 http://download.intel.com/design/intarch/papers/321071.pdf

I'm not sure how to explain the PCI-express chip bottleneck I'm
seeing, but my guess is that I'm limited by the number of outstanding
packets/DMA-transfers and the latency for the DMA operations.

Does any one have datasheets on the X58 and NVIDIA's NF200 PCI-express
chips, that can tell me the number of outstanding transfers they
support?

-- 
Med venlig hilsen / Best regards
  Jesper Brouer
  ComX Networks A/S
  Linux Network developer
  Cand. Scient Datalog / MSc.
  Author of http://adsl-optimizer.dk
  LinkedIn: http://www.linkedin.com/in/brouer


^ permalink raw reply

* [PATCH] net: explain netns notifiers a little better
From: Johannes Berg @ 2009-07-15 16:16 UTC (permalink / raw)
  To: netdev; +Cc: Eric W. Biederman

Eric explained this to me -- and afterwards the comment
made sense, but not before. Add the the critical point
about interfaces having to be gone from the netns before
subsys notifiers are called.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 include/net/net_namespace.h |   16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

--- wireless-testing.orig/include/net/net_namespace.h	2009-07-11 11:18:20.000000000 +0200
+++ wireless-testing/include/net/net_namespace.h	2009-07-11 11:20:49.000000000 +0200
@@ -239,13 +239,15 @@ struct pernet_operations {
  * needs per network namespace operations use device pernet operations,
  * otherwise use pernet subsys operations.
  *
- * This is critically important.  Most of the network code cleanup
- * runs with the assumption that dev_remove_pack has been called so no
- * new packets will arrive during and after the cleanup functions have
- * been called.  dev_remove_pack is not per namespace so instead the
- * guarantee of no more packets arriving in a network namespace is
- * provided by ensuring that all network devices and all sockets have
- * left the network namespace before the cleanup methods are called.
+ * Network interfaces need to be removed from a dying netns _before_
+ * subsys notifiers can be called, as most of the network code cleanup
+ * (which is done from subsys notifiers) runs with the assumption that
+ * dev_remove_pack has been called so no new packets will arrive during
+ * and after the cleanup functions have been called.  dev_remove_pack
+ * is not per namespace so instead the guarantee of no more packets
+ * arriving in a network namespace is provided by ensuring that all
+ * network devices and all sockets have left the network namespace
+ * before the cleanup methods are called.
  *
  * For the longest time the ipv4 icmp code was registered as a pernet
  * device which caused kernel oops, and panics during network



^ permalink raw reply

* Re: [PATCH 3/3 v3] net/compat/wext: send different messages to compat tasks
From: David Miller @ 2009-07-15 16:08 UTC (permalink / raw)
  To: johannes-cdvu00un1VgdHxzADdlk8Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1246483562.8154.11.camel-YfaajirXv2244ywRPIzf9A@public.gmane.org>


Ok, all 3 patches applied to net-next-2.6, let's see what explodes ;-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/1] veth: don't free priv->status until dev->destructor (v2)
From: David Miller @ 2009-07-15 15:50 UTC (permalink / raw)
  To: serue; +Cc: shemminger, containers, sachinp, netdev, matthltc, linux-kernel
In-Reply-To: <20090626162418.GA24828@us.ibm.com>

From: "Serge E. Hallyn" <serue@us.ibm.com>
Date: Fri, 26 Jun 2009 11:24:18 -0500

> I haven't been able to reproduce the original oops though (been
> trying to cat the stats sysfs files while rmmoding veth, to no
> avail, and haven't found an original bug report or testcase), so
> can't verify whether this patch prevents the original oops.

If you 'cat' it you're unlikely to trigger the oops.

You have to hold the sysfs files open, and then elsewhere do the
rmmod, wait, and then continue with some access to those open sysfs
file descriptors (f.e. do some reads).

I'd also need this patch to be against current sources as they'll
never apply since I did the revert quite some time ago.

Thanks.

^ permalink raw reply

* [PATCH] net: deprecate print_mac
From: Johannes Berg @ 2009-07-15 15:23 UTC (permalink / raw)
  To: netdev

We've had %pM for long enough now, time to deprecate
print_mac() and remove the __maybe_unused attribute
from DECLARE_MAC_BUF so that variables declared with
that can be found and removed. Otherwise people are
putting in new users of print_mac().

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
On my tree, with three other patches I have submitted
to gianfar, fcoe and wireless, there's now no user of
DECLARE_MAC_BUF() left. I'm open to just killing it
right away, but that seems like a pain to coordinate
and would likely get merge conflicts, so this is much
simpler and then killing it next cycle.

 include/linux/if_ether.h |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- wireless-testing.orig/include/linux/if_ether.h	2009-07-15 17:13:31.000000000 +0200
+++ wireless-testing/include/linux/if_ether.h	2009-07-15 17:13:36.000000000 +0200
@@ -138,10 +138,10 @@ extern ssize_t sysfs_format_mac(char *bu
 /*
  *	Display a 6 byte device address (MAC) in a readable format.
  */
-extern char *print_mac(char *buf, const unsigned char *addr);
+extern char *print_mac(char *buf, const unsigned char *addr) __deprecated;
 #define MAC_FMT "%02x:%02x:%02x:%02x:%02x:%02x"
 #define MAC_BUF_SIZE	18
-#define DECLARE_MAC_BUF(var) char var[MAC_BUF_SIZE] __maybe_unused
+#define DECLARE_MAC_BUF(var) char var[MAC_BUF_SIZE]
 
 #endif
 



^ permalink raw reply

* Re: [PATCH] net: nf_conntrack_alloc() should not use kmem_cache_zalloc()
From: Patrick McHardy @ 2009-07-15 15:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, paulmck
In-Reply-To: <4A5DCB7C.9000502@gmail.com>

Eric Dumazet wrote:
> [PATCH] net: nf_conntrack_alloc() should not use kmem_cache_zalloc()
> 
> When a slab cache uses SLAB_DESTROY_BY_RCU, we must be careful when allocating
> objects, since slab allocator could give a freed object still used by lockless
> readers.
> 
> In particular, nf_conntrack RCU lookups rely on ct->tuplehash[xxx].hnnode.next
> being always valid (ie containing a valid 'nulls' value, or a valid pointer to next
> object in hash chain.)
> 
> kmem_cache_zalloc() setups object with NULL values, but a NULL value is not valid
> for ct->tuplehash[xxx].hnnode.next.
> 
> Fix is to call kmem_cache_alloc() and do the zeroing ourself.

I think this is still racy, please see below:

> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index 7508f11..23feafa 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -561,17 +561,28 @@ struct nf_conn *nf_conntrack_alloc(struct net *net,
>  		}
>  	}
>  
> -	ct = kmem_cache_zalloc(nf_conntrack_cachep, gfp);
> +	/*
> +	 * Do not use kmem_cache_zalloc(), as this cache uses
> +	 * SLAB_DESTROY_BY_RCU.
> +	 */
> +	ct = kmem_cache_alloc(nf_conntrack_cachep, gfp);
>  	if (ct == NULL) {
>  		pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n");
>  		atomic_dec(&net->ct.count);
>  		return ERR_PTR(-ENOMEM);
>  	}
> -

__nf_conntrack_find() on another CPU finds the entry at this point.

> +	/*
> +	 * Let ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.next
> +	 * and ct->tuplehash[IP_CT_DIR_REPLY].hnnode.next unchanged.
> +	 */
> +	memset(&ct->tuplehash[IP_CT_DIR_MAX], 0,
> +	       sizeof(*ct) - offsetof(struct nf_conn, tuplehash[IP_CT_DIR_MAX]));
>  	spin_lock_init(&ct->lock);
>  	atomic_set(&ct->ct_general.use, 1);

nf_conntrack_find_get() successfully tries to atomic_inc_not_zero()
at this point, following by another tuple comparison which is also
successful.

Am I missing something? I think we need to make sure the reference
count is not increased until the new tuples are visible.

>  	ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
> +	ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
>  	ct->tuplehash[IP_CT_DIR_REPLY].tuple = *repl;
> +	ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev = NULL;
>  	/* Don't set timer yet: wait for confirmation */
>  	setup_timer(&ct->timeout, death_by_timeout, (unsigned long)ct);
>  #ifdef CONFIG_NET_NS
> 


^ permalink raw reply

* [PATCH] gianfar: remove unused DECLARE_MAC_BUF
From: Johannes Berg @ 2009-07-15 15:18 UTC (permalink / raw)
  To: netdev; +Cc: Kumar Gala

The use of it was converted to %pM, but the variable
stuck -- remove it now to not cause spurious warnings.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 drivers/net/gianfar.c |    1 -
 1 file changed, 1 deletion(-)

--- wireless-testing.orig/drivers/net/gianfar.c	2009-07-15 17:11:36.000000000 +0200
+++ wireless-testing/drivers/net/gianfar.c	2009-07-15 17:11:43.000000000 +0200
@@ -304,7 +304,6 @@ static int gfar_probe(struct of_device *
 	u32 tempval;
 	struct net_device *dev = NULL;
 	struct gfar_private *priv = NULL;
-	DECLARE_MAC_BUF(mac);
 	int err = 0;
 	int len_devname;
 



^ permalink raw reply

* Re: [PATCH] iproute2 flush: handle larger tables and deleted entries
From: Patrick McHardy @ 2009-07-15 15:19 UTC (permalink / raw)
  To: Gautam Kachroo; +Cc: netdev
In-Reply-To: <4e0db5bc0907140945i3190cfb7g7b3e6a0f1c10bc8a@mail.gmail.com>

Gautam Kachroo wrote:
> On Tue, Jul 14, 2009 at 2:38 AM, Patrick McHardy<kaber@trash.net> wrote:
>> Gautam Kachroo wrote:
>>> use a new netlink socket when sending flush messages to avoid reading
>>> any pending data on the existing netlink socket.
>>>
>>> read all of the response from the netlink request -- this response can
>>> be split over multiple recv calls, pretty much one per netlink request
>>> message. ENOENT errors, which correspond to attempts to delete an
>>> already deleted entry, are ignored. Other errors are not ignored.
>>
>> In which case would there be any pending data? From what I can see,
>> this can only happen when using batching, but in that case the
>> previous command should continue reading until it has received all
>> responses (which the netlink functions appear to be doing properly).
> 
> What is the "previous command"?

The last command before the one executing when using batching.

> Are you referring to rtnl_dump_filter? If rtnl_send_check comes across
> a failure, rtnl_dump_filter will not continue reading.
> 
> Here's the situation that I'm referring to:
> 
> If rtnl_send_check detects an error, it returns -1. rtnl_send_check is
> called from flush_update. The multiple implementations of flush_update
> (e.g. in ipneigh.c, ipaddress.c) propagate this return value to their
> caller, e.g. print_neigh or print_addrinfo.
> 
> print_neigh, print_addrinfo, etc. are called from rtnl_dump_filter.
> rtnl_dump_filter sits in a loop calling recvmsg on the netlink socket.
> However, it returns the error value if the filter function (e.g.
> print_neigh) returns an error. In this case, rtnl_dump_filter can
> return before it's read all the responses.
> The error return from rtnl_dump_filter causes the program to exit.

Yes, and I agree with your patch so far. My question is why you
need another socket.

> use a new netlink socket when sending flush messages to avoid reading
> any pending data on the existing netlink socket.

Under what circumstances would there be pending data when
performing a new iproute operation?

^ permalink raw reply

* [PATCH 2/2] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfgang Denk @ 2009-07-15 15:18 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Wolfgang Denk, Grant Likely, Kumar Gala, netdev
In-Reply-To: <1247578966-9847-1-git-send-email-wd@denx.de>

This patch adds error checking and prevents clobbering unrelated bits
(reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
register on MPC52xx systems.

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: <netdev@vger.kernel.org>
---
 drivers/net/fec_mpc52xx.c     |    2 +-
 drivers/net/fec_mpc52xx_phy.c |    6 ++++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index cc78633..b69d440 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
 	/* set phy speed.
 	 * this can't be done in phy driver, since it needs to be called
 	 * before fec stuff (even on resume) */
-	out_be32(&fec->mii_speed, priv->mdio_speed);
+	clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
 }
 
 /**
diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
index 31e6d62..f733d43 100644
--- a/drivers/net/fec_mpc52xx_phy.c
+++ b/drivers/net/fec_mpc52xx_phy.c
@@ -105,8 +105,10 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
 	dev_set_drvdata(dev, bus);
 
 	/* set MII speed */
-	out_be32(&priv->regs->mii_speed,
-		((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
+	i = mpc5xxx_get_mii_speed(of);
+	if (i<0)
+		goto out_unmap;
+	clrsetbits_be32(&priv->regs->mii_speed, 0x7E, i);
 
 	err = of_mdiobus_register(bus, np);
 	if (err)
-- 
1.6.0.6


^ permalink raw reply related

* [PATCH 1/2 v3] fs_enet/mii-fec.c: fix MII speed calculation
From: Wolfgang Denk @ 2009-07-15 15:18 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: netdev, Wolfgang Denk
In-Reply-To: <1247578966-9847-1-git-send-email-wd@denx.de>

The MII speed calculation was based on the CPU clock (ppc_proc_freq),
but for MPC512x we must use the bus clock instead.

This patch makes it use the correct clock and makes sure we don't
clobber reserved bits in the MII_SPEED register.

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: <netdev@vger.kernel.org>
---
Please ignore patch v2, it's crap.
Hope this is a bit better.

 arch/powerpc/include/asm/mpc5xxx.h   |   10 +++++++++
 arch/powerpc/sysdev/mpc5xxx_clocks.c |   37 ++++++++++++++++++++++++++++++++++
 drivers/net/fs_enet/mii-fec.c        |   13 +++++++++--
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/include/asm/mpc5xxx.h b/arch/powerpc/include/asm/mpc5xxx.h
index 5ce9c5f..86ab29f 100644
--- a/arch/powerpc/include/asm/mpc5xxx.h
+++ b/arch/powerpc/include/asm/mpc5xxx.h
@@ -15,8 +15,18 @@
 
 #ifndef __ASM_POWERPC_MPC5xxx_H__
 #define __ASM_POWERPC_MPC5xxx_H__
+#include <linux/of_platform.h>
 
 extern unsigned long mpc5xxx_get_bus_frequency(struct device_node *node);
 
+#if defined(CONFIG_PPC_MPC512x) || defined(CONFIG_PPC_MPC52xx)
+extern int mpc5xxx_get_mii_speed(struct of_device *ofdev);
+#else
+static inline int mpc5xxx_get_mii_speed(struct of_device *ofdev)
+{
+	return -1;
+}
+#endif
+
 #endif /* __ASM_POWERPC_MPC5xxx_H__ */
 
diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c
index 34e12f9..e26d12b 100644
--- a/arch/powerpc/sysdev/mpc5xxx_clocks.c
+++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c
@@ -31,3 +31,40 @@ mpc5xxx_get_bus_frequency(struct device_node *node)
 	return p_bus_freq ? *p_bus_freq : 0;
 }
 EXPORT_SYMBOL(mpc5xxx_get_bus_frequency);
+
+/**
+ *	mpc5xxx_get_mii_speed - Get the MII_SPEED value
+ *	@node:	device node
+ *
+ *	Returns the MII_SPEED value for MPC512x and MPC52xx systems.
+ *	The value gets computed such that the resulting MDC frequency
+ *	is 2.5 MHz or lower.
+ */
+
+int
+mpc5xxx_get_mii_speed(struct of_device *ofdev)
+{
+	unsigned int clock, speed;
+
+	clock = mpc5xxx_get_bus_frequency(ofdev->node);
+
+	if (!clock) {
+		dev_err(&ofdev->dev, "could not determine IPS/IPB clock\n");
+		return -ENODEV;
+	}
+
+	/* scale for a MII clock <= 2.5 MHz */
+	speed = (clock + 2499999) / 2500000;
+
+	/* only 6 bits available for MII speed */
+	if (speed > 0x3F) {
+		speed = 0x3F;
+		dev_err(&ofdev->dev,
+			"MII clock (%d MHz) exceeds max (2.5 MHz)\n",
+			clock / speed);
+	}
+
+	/* Field is in bits 25:30 of MII_SPEED register */
+	return speed << 1;
+}
+EXPORT_SYMBOL(mpc5xxx_get_mii_speed);
diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.c
index 75a0999..a28d39f 100644
--- a/drivers/net/fs_enet/mii-fec.c
+++ b/drivers/net/fs_enet/mii-fec.c
@@ -36,6 +36,7 @@
 #include <asm/pgtable.h>
 #include <asm/irq.h>
 #include <asm/uaccess.h>
+#include <asm/mpc5xxx.h>
 
 #include "fs_enet.h"
 #include "fec.h"
@@ -103,7 +104,6 @@ static int fs_enet_fec_mii_reset(struct mii_bus *bus)
 static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
                                         const struct of_device_id *match)
 {
-	struct device_node *np = NULL;
 	struct resource res;
 	struct mii_bus *new_bus;
 	struct fec_info *fec;
@@ -133,13 +133,20 @@ static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
 	if (!fec->fecp)
 		goto out_fec;
 
-	fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
+	if (of_device_is_compatible(ofdev->node, "fsl,mpc5121-fec-mdio")) {
+		i = mpc5xxx_get_mii_speed(ofdev);
+		if (i < 0)
+			goto out_unmap_regs;
+		fec->mii_speed = i;
+	} else {
+		fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
+	}
 
 	setbits32(&fec->fecp->fec_r_cntrl, FEC_RCNTRL_MII_MODE);
 	setbits32(&fec->fecp->fec_ecntrl, FEC_ECNTRL_PINMUX |
 	                                  FEC_ECNTRL_ETHER_EN);
 	out_be32(&fec->fecp->fec_ievent, FEC_ENET_MII);
-	out_be32(&fec->fecp->fec_mii_speed, fec->mii_speed);
+	clrsetbits_be32(&fec->fecp->fec_mii_speed, 0x7E, fec->mii_speed);
 
 	new_bus->phy_mask = ~0;
 	new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
-- 
1.6.0.6

^ permalink raw reply related

* Re: [PATCH] netfilter: ebtables: Use print_mac instead of own function
From: Patrick McHardy @ 2009-07-15 15:13 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: bart.de.schuymer, shemminger, davem, ebtables-devel,
	netfilter-devel, netdev
In-Reply-To: <1247655306-21337-1-git-send-email-klto@zhaw.ch>

Tobias Klauser wrote:
> ebt_log uses its own implementation of print_mac to print MAC addresses.
> This patch converts it to use print_mac from linux/if_ether.h

You can do this even simpler by using %pM as format string.


^ permalink raw reply

* Re: [PATCH] bonding: clean muticast addresses when bevice changes type
From: Moni Shoua @ 2009-07-15 15:03 UTC (permalink / raw)
  To: Moni Shoua
  Cc: Jay Vosburgh, David Miller, Roland Dreier, bonding-devel, netdev
In-Reply-To: <4A5DEE1F.4090707@Voltaire.COM>

I forgot to mention that the need for this patch was raised here
http://lists.openfabrics.org/pipermail/general/2009-July/060496.html


^ permalink raw reply

* [PATCH] bonding: clean muticast addresses when bevice changes type
From: Moni Shoua @ 2009-07-15 14:56 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: David Miller, Roland Dreier, bonding-devel, netdev

Bonding device  forbids slave device of different types under the same master.
However, it is possible for a bonding master to change  type during its lifetime. 
This can be either from ARPHRD_ETHER to ARPHRD_INFINIBAND or the other way arround.  
The change of type requires device level multicast address cleanup because device
level multicast addresses depend on the device type. 

The patch adds a call to dev_close() before the bonding master changes type and 
dev_open() just after that.

In the example below I enslaved an IPoIB device (ib0) under bond0. Since
each bonding master starts as device of type ARPHRD_ETHER by default, a change
of type occurs when ib0 is enslaved.

This is how /proc/net/dev_mcast looks like without the patch

5    bond0           1     0     00ffffffff12601bffff000000000001ff96ca05
5    bond0           1     0     01005e000116
5    bond0           1     0     01005e7ffffd
5    bond0           1     0     01005e000001
5    bond0           1     0     333300000001
6    ib0             1     0     00ffffffff12601bffff000000000001ff96ca05
6    ib0             1     0     333300000001
6    ib0             1     0     01005e000001
6    ib0             1     0     01005e7ffffd
6    ib0             1     0     01005e000116
6    ib0             1     0     00ffffffff12401bffff00000000000000000001
6    ib0             1     0     00ffffffff12601bffff00000000000000000001

and this is how it looks like after the patch.

5    bond0           1     0     00ffffffff12601bffff000000000001ff96ca05
5    bond0           1     0     00ffffffff12601bffff00000000000000000001
5    bond0           1     0     00ffffffff12401bffff0000000000000ffffffd
5    bond0           1     0     00ffffffff12401bffff00000000000000000116
5    bond0           1     0     00ffffffff12401bffff00000000000000000001
6    ib0             1     0     00ffffffff12601bffff000000000001ff96ca05
6    ib0             1     0     00ffffffff12401bffff00000000000000000116
6    ib0             1     0     00ffffffff12401bffff0000000000000ffffffd
6    ib0             2     0     00ffffffff12401bffff00000000000000000001
6    ib0             2     0     00ffffffff12601bffff00000000000000000001

Signed-off-by: Moni Shoua <monis@voltaire.com>
--

 drivers/net/bonding/bond_main.c |   12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index d927f71..aa1be1f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1459,8 +1459,16 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
 	 */
 	if (bond->slave_cnt == 0) {
-		if (slave_dev->type != ARPHRD_ETHER)
-			bond_setup_by_slave(bond_dev, slave_dev);
+		if (bond_dev->type != slave_dev->type) {
+			dev_close(bond_dev);
+			pr_debug("%s: change device type from %d to %d\n",
+				bond_dev->name, bond_dev->type, slave_dev->type);
+			if (slave_dev->type != ARPHRD_ETHER)
+				bond_setup_by_slave(bond_dev, slave_dev);
+			else
+				ether_setup(bond_dev);
+			dev_open(bond_dev);
+		}
 	} else if (bond_dev->type != slave_dev->type) {
 		pr_err(DRV_NAME ": %s ether type (%d) is different "
 			"from other slaves (%d), can not enslave it.\n",

^ permalink raw reply related

* Re: [PATCH] USB host CDC Phonet network interface driver
From: Oliver Neukum @ 2009-07-15 14:20 UTC (permalink / raw)
  To: Rémi Denis-Courmont
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1247666341-7121-1-git-send-email-remi.denis-courmont-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org>

Am Mittwoch, 15. Juli 2009 15:59:01 schrieb Rémi Denis-Courmont:

Hello,

> +	netif_stop_queue(dev);
> +	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
> +		netif_wake_queue(dev);

This is unelegant.


> +	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
> +	if (err)
> +		return err;
> +
> +	for (i = 0; i < rxq_size; i++) {
> +		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
> +
> +		if (req && rx_submit(pnd, req, GFP_KERNEL)) {
> +			usb_free_urb(req);
> +			req = NULL;
> +		}
> +		pnd->urbs[i] = req;
> +	}

You should fail if you cannot get a minimum number of URBs running.
> +
> +	netif_wake_queue(dev);
> +	return 0;
> +}
[..]
> +	/* Endpoints */
> +	if ((data_desc->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) ==
> +			USB_DIR_IN) {

Please use the macro.

> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +	} else {
> +		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
> +			data_desc->endpoint[1].desc.bEndpointAddress);
> +		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
> +			data_desc->endpoint[0].desc.bEndpointAddress);
> +	}
> +	pnd->active_setting = data_desc - data_intf->altsetting;
> +
> +	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
> +	if (err)
> +		goto out;
> +
> +	/* Force inactive mode until the network device is brought UP */
> +	usb_set_interface(usbdev, union_header->bSlaveInterface0,
> +				!pnd->active_setting);
> +	usb_set_intfdata(intf, pnd);
> +	BUG_ON(!usb_get_intfdata(intf));

What for?

> +
> +	err = register_netdev(dev);
> +	if (err) {
> +		usb_driver_release_interface(&usbpn_driver, data_intf);
> +		goto out;
> +	}
> +
> +	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
> +	return 0;
> +
> +out:
> +	usb_set_intfdata(intf, NULL);
> +	free_netdev(dev);
> +	return err;
> +}
> +
> +static void usbpn_disconnect(struct usb_interface *intf)
> +{
> +	struct usbpn_dev *pnd = usb_get_intfdata(intf);
> +

What happens if this happens in the unexpected order?

> +	if (intf != pnd->data_intf) {
> +		usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
> +		return;
> +	}
> +
> +	unregister_netdev(pnd->dev);
> +	usb_put_dev(pnd->usb);
> +}

	Regards
		Oliver

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] USB host CDC Phonet network interface driver
From: Rémi Denis-Courmont @ 2009-07-15 13:59 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb

From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Many Nokia handsets support a Phonet interface to the cellular modem
via a vendor-specific USB interface. CDC Phonet follows the
Communications Device Class model, with one control interface, and
and a pair of inactive and active data alternative interface. The later
has two bulk endpoint, one per direction.

This was tested against Nokia E61, Nokia N95, and the existing Phonet
gadget function for the Linux composite USB gadget framework.

Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 drivers/net/usb/Kconfig      |    8 +
 drivers/net/usb/Makefile     |    1 +
 drivers/net/usb/cdc-phonet.c |  451 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 460 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/usb/cdc-phonet.c

diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index a906d39..c47237c 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -369,4 +369,12 @@ config USB_NET_INT51X1
 	  (Powerline Communications) solution with an Intellon
 	  INT51x1/INT5200 chip, like the "devolo dLan duo".
 
+config USB_CDC_PHONET
+	tristate "CDC Phonet support"
+	depends on PHONET
+	help
+	  Choose this option to support the Phonet interface to a Nokia
+	  cellular modem, as found on most Nokia handsets with the
+	  "PC suite" USB profile.
+
 endmenu
diff --git a/drivers/net/usb/Makefile b/drivers/net/usb/Makefile
index b870b0b..e17afb7 100644
--- a/drivers/net/usb/Makefile
+++ b/drivers/net/usb/Makefile
@@ -21,4 +21,5 @@ obj-$(CONFIG_USB_NET_ZAURUS)	+= zaurus.o
 obj-$(CONFIG_USB_NET_MCS7830)	+= mcs7830.o
 obj-$(CONFIG_USB_USBNET)	+= usbnet.o
 obj-$(CONFIG_USB_NET_INT51X1)	+= int51x1.o
+obj-$(CONFIG_USB_CDC_PHONET)	+= cdc-phonet.o
 
diff --git a/drivers/net/usb/cdc-phonet.c b/drivers/net/usb/cdc-phonet.c
new file mode 100644
index 0000000..d335b72
--- /dev/null
+++ b/drivers/net/usb/cdc-phonet.c
@@ -0,0 +1,451 @@
+/*
+ * phonet.c -- USB CDC Phonet host driver
+ *
+ * Copyright (C) 2008-2009 Nokia Corporation. All rights reserved.
+ *
+ * Author: Rémi Denis-Courmont
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
+ * 02110-1301 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/usb.h>
+#include <linux/usb/cdc.h>
+#include <linux/netdevice.h>
+#include <linux/if_arp.h>
+#include <linux/if_phonet.h>
+
+#define PN_MEDIA_USB	0x1B
+
+static const unsigned rxq_size = 17;
+
+struct usbpn_dev {
+	struct net_device	*dev;
+	atomic_t		tx_queue;
+
+	struct usb_interface	*intf, *data_intf;
+	struct usb_device	*usb;
+	unsigned int		tx_pipe, rx_pipe;
+	u8 active_setting;
+
+	spinlock_t		rx_lock;
+	struct sk_buff		*rx_skb;
+	struct urb		*urbs[0];
+};
+
+static void tx_complete(struct urb *req);
+static void rx_complete(struct urb *req);
+
+/*
+ * Network device callbacks
+ */
+static int usbpn_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct urb *req = NULL;
+	int err;
+
+	if (skb->protocol != htons(ETH_P_PHONET))
+		goto drop;
+
+	req = usb_alloc_urb(0, GFP_ATOMIC);
+	if (!req)
+		goto drop;
+	usb_fill_bulk_urb(req, pnd->usb, pnd->tx_pipe, skb->data, skb->len,
+				tx_complete, skb);
+	req->transfer_flags = URB_ZERO_PACKET;
+	err = usb_submit_urb(req, GFP_ATOMIC);
+	if (err) {
+		usb_free_urb(req);
+		goto drop;
+	}
+
+	netif_stop_queue(dev);
+	if (atomic_inc_return(&pnd->tx_queue) < dev->tx_queue_len)
+		netif_wake_queue(dev);
+	return 0;
+
+drop:
+	dev_kfree_skb(skb);
+	dev->stats.tx_dropped++;
+	return 0;
+}
+
+static void tx_complete(struct urb *req)
+{
+	struct sk_buff *skb = req->context;
+	struct net_device *dev = skb->dev;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+
+	switch (req->status) {
+	case 0:
+		dev->stats.tx_bytes += skb->len;
+		break;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		dev->stats.tx_aborted_errors++;
+	default:
+		dev->stats.tx_errors++;
+		dev_dbg(&dev->dev, "TX error (%d)\n", req->status);
+	}
+	dev->stats.tx_packets++;
+
+	atomic_dec(&pnd->tx_queue);
+	netif_wake_queue(dev);
+
+	dev_kfree_skb_any(skb);
+	usb_free_urb(req);
+}
+
+static int rx_submit(struct usbpn_dev *pnd, struct urb *req, gfp_t gfp_flags)
+{
+	struct net_device *dev = pnd->dev;
+	struct page *page;
+	int err;
+
+	page = __netdev_alloc_page(dev, gfp_flags);
+	if (!page)
+		return -ENOMEM;
+
+	usb_fill_bulk_urb(req, pnd->usb, pnd->rx_pipe, page_address(page),
+				PAGE_SIZE, rx_complete, dev);
+	req->transfer_flags = 0;
+	err = usb_submit_urb(req, gfp_flags);
+	if (unlikely(err)) {
+		dev_dbg(&dev->dev, "RX submit error (%d)\n", err);
+		netdev_free_page(dev, page);
+	}
+	return err;
+}
+
+static void rx_complete(struct urb *req)
+{
+	struct net_device *dev = req->context;
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	struct page *page = virt_to_page(req->transfer_buffer);
+	struct sk_buff *skb;
+	unsigned long flags;
+
+	switch (req->status) {
+	case 0:
+		spin_lock_irqsave(&pnd->rx_lock, flags);
+		skb = pnd->rx_skb;
+		if (!skb) {
+			skb = pnd->rx_skb = netdev_alloc_skb(dev, 12);
+			if (likely(skb)) {
+				/* Can't use pskb_pull() on page in IRQ */
+				memcpy(skb_put(skb, 1), page_address(page), 1);
+				skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+						page, 1, req->actual_length);
+				page = NULL;
+			}
+		} else {
+			skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
+					page, 0, req->actual_length);
+			page = NULL;
+		}
+		if (req->actual_length < PAGE_SIZE)
+			pnd->rx_skb = NULL; /* Last fragment */
+		else
+			skb = NULL;
+		spin_unlock_irqrestore(&pnd->rx_lock, flags);
+		if (skb) {
+			skb->protocol = htons(ETH_P_PHONET);
+			skb_reset_mac_header(skb);
+			__skb_pull(skb, 1);
+			skb->dev = dev;
+			dev->stats.rx_packets++;
+			dev->stats.rx_bytes += skb->len;
+
+			netif_rx(skb);
+		}
+		goto resubmit;
+
+	case -ENOENT:
+	case -ECONNRESET:
+	case -ESHUTDOWN:
+		req = NULL;
+		break;
+
+	case -EOVERFLOW:
+		dev->stats.rx_over_errors++;
+		dev_dbg(&dev->dev, "RX overflow\n");
+		break;
+
+	case -EILSEQ:
+		dev->stats.rx_crc_errors++;
+		break;
+	}
+
+	dev->stats.rx_errors++;
+resubmit:
+	if (page)
+		netdev_free_page(dev, page);
+	if (req)
+		rx_submit(pnd, req, GFP_ATOMIC);
+}
+
+static int usbpn_open(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	int err;
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	err = usb_set_interface(pnd->usb, num, pnd->active_setting);
+	if (err)
+		return err;
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = usb_alloc_urb(0, GFP_KERNEL);
+
+		if (req && rx_submit(pnd, req, GFP_KERNEL)) {
+			usb_free_urb(req);
+			req = NULL;
+		}
+		pnd->urbs[i] = req;
+	}
+
+	netif_wake_queue(dev);
+	return 0;
+}
+
+static int usbpn_close(struct net_device *dev)
+{
+	struct usbpn_dev *pnd = netdev_priv(dev);
+	unsigned i;
+	unsigned num = pnd->data_intf->cur_altsetting->desc.bInterfaceNumber;
+
+	netif_stop_queue(dev);
+
+	for (i = 0; i < rxq_size; i++) {
+		struct urb *req = pnd->urbs[i];
+
+		if (!req)
+			continue;
+		usb_kill_urb(req);
+		usb_free_urb(req);
+		pnd->urbs[i] = NULL;
+	}
+
+	return usb_set_interface(pnd->usb, num, !pnd->active_setting);
+}
+
+static int usbpn_set_mtu(struct net_device *dev, int new_mtu)
+{
+	if ((new_mtu < PHONET_MIN_MTU) || (new_mtu > PHONET_MAX_MTU))
+		return -EINVAL;
+
+	dev->mtu = new_mtu;
+	return 0;
+}
+
+static const struct net_device_ops usbpn_ops = {
+	.ndo_open	= usbpn_open,
+	.ndo_stop	= usbpn_close,
+	.ndo_start_xmit = usbpn_xmit,
+	.ndo_change_mtu = usbpn_set_mtu,
+};
+
+static void usbpn_setup(struct net_device *dev)
+{
+	dev->features		= 0;
+	dev->netdev_ops		= &usbpn_ops,
+	dev->header_ops		= &phonet_header_ops;
+	dev->type		= ARPHRD_PHONET;
+	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		= PHONET_MAX_MTU;
+	dev->hard_header_len	= 1;
+	dev->dev_addr[0]	= PN_MEDIA_USB;
+	dev->addr_len		= 1;
+	dev->tx_queue_len	= 3;
+
+	dev->destructor		= free_netdev;
+}
+
+/*
+ * USB driver callbacks
+ */
+static struct usb_device_id usbpn_ids[] = {
+	{
+		.match_flags = USB_DEVICE_ID_MATCH_VENDOR
+			| USB_DEVICE_ID_MATCH_INT_CLASS
+			| USB_DEVICE_ID_MATCH_INT_SUBCLASS,
+		.idVendor = 0x0421, /* Nokia */
+		.bInterfaceClass = USB_CLASS_COMM,
+		.bInterfaceSubClass = 0xFE,
+	},
+	{ },
+};
+
+MODULE_DEVICE_TABLE(usb, usbpn_ids);
+
+static struct usb_driver usbpn_driver;
+
+int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
+{
+	static const char ifname[] = "usbpn%d";
+	const struct usb_cdc_union_desc *union_header = NULL;
+	const struct usb_cdc_header_desc *phonet_header = NULL;
+	const struct usb_host_interface *data_desc;
+	struct usb_interface *data_intf;
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+	struct net_device *dev;
+	struct usbpn_dev *pnd;
+	u8 *data;
+	int len, err;
+
+	data = intf->altsetting->extra;
+	len = intf->altsetting->extralen;
+	while (len >= 3) {
+		u8 dlen = data[0];
+		if (dlen < 3)
+			return -EINVAL;
+
+		/* bDescriptorType */
+		if (data[1] == USB_DT_CS_INTERFACE) {
+			/* bDescriptorSubType */
+			switch (data[2]) {
+			case USB_CDC_UNION_TYPE:
+				if (union_header || dlen < 5)
+					break;
+				union_header =
+					(struct usb_cdc_union_desc *)data;
+				break;
+			case 0xAB:
+				if (phonet_header || dlen < 5)
+					break;
+				phonet_header =
+					(struct usb_cdc_header_desc *)data;
+				break;
+			}
+		}
+		data += dlen;
+		len -= dlen;
+	}
+
+	if (!union_header || !phonet_header)
+		return -EINVAL;
+
+	data_intf = usb_ifnum_to_if(usbdev, union_header->bSlaveInterface0);
+	if (data_intf == NULL)
+		return -ENODEV;
+	/* Data interface has one inactive and one active setting */
+	if (data_intf->num_altsetting != 2)
+		return -EINVAL;
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 0
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 2)
+		data_desc = data_intf->altsetting + 1;
+	else
+	if (data_intf->altsetting[0].desc.bNumEndpoints == 2
+	 && data_intf->altsetting[1].desc.bNumEndpoints == 0)
+		data_desc = data_intf->altsetting;
+	else
+		return -EINVAL;
+
+	dev = alloc_netdev(sizeof(*pnd) + sizeof(pnd->urbs[0]) * rxq_size,
+				ifname, usbpn_setup);
+	if (!dev)
+		return -ENOMEM;
+
+	pnd = netdev_priv(dev);
+	SET_NETDEV_DEV(dev, &intf->dev);
+	netif_stop_queue(dev);
+
+	pnd->dev = dev;
+	atomic_set(&pnd->tx_queue, 0);
+	pnd->usb = usb_get_dev(usbdev);
+	pnd->intf = intf;
+	pnd->data_intf = data_intf;
+	spin_lock_init(&pnd->rx_lock);
+	/* Endpoints */
+	if ((data_desc->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) ==
+			USB_DIR_IN) {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+	} else {
+		pnd->rx_pipe = usb_rcvbulkpipe(usbdev,
+			data_desc->endpoint[1].desc.bEndpointAddress);
+		pnd->tx_pipe = usb_sndbulkpipe(usbdev,
+			data_desc->endpoint[0].desc.bEndpointAddress);
+	}
+	pnd->active_setting = data_desc - data_intf->altsetting;
+
+	err = usb_driver_claim_interface(&usbpn_driver, data_intf, pnd);
+	if (err)
+		goto out;
+
+	/* Force inactive mode until the network device is brought UP */
+	usb_set_interface(usbdev, union_header->bSlaveInterface0,
+				!pnd->active_setting);
+	usb_set_intfdata(intf, pnd);
+	BUG_ON(!usb_get_intfdata(intf));
+
+	err = register_netdev(dev);
+	if (err) {
+		usb_driver_release_interface(&usbpn_driver, data_intf);
+		goto out;
+	}
+
+	dev_dbg(&dev->dev, "USB CDC Phonet device found\n");
+	return 0;
+
+out:
+	usb_set_intfdata(intf, NULL);
+	free_netdev(dev);
+	return err;
+}
+
+static void usbpn_disconnect(struct usb_interface *intf)
+{
+	struct usbpn_dev *pnd = usb_get_intfdata(intf);
+
+	if (intf != pnd->data_intf) {
+		usb_driver_release_interface(&usbpn_driver, pnd->data_intf);
+		return;
+	}
+
+	unregister_netdev(pnd->dev);
+	usb_put_dev(pnd->usb);
+}
+
+static struct usb_driver usbpn_driver = {
+	.name =		"cdc_phonet",
+	.probe =	usbpn_probe,
+	.disconnect =	usbpn_disconnect,
+	.id_table =	usbpn_ids,
+};
+
+static int __init usbpn_init(void)
+{
+	return usb_register(&usbpn_driver);
+}
+
+static void __exit usbpn_exit(void)
+{
+	usb_deregister(&usbpn_driver);
+}
+
+module_init(usbpn_init);
+module_exit(usbpn_exit);
+
+MODULE_AUTHOR("Remi Denis-Courmont");
+MODULE_DESCRIPTION("USB CDC Phonet host interface");
+MODULE_LICENSE("GPL");
-- 
1.6.0.4


^ permalink raw reply related

* Re: [PATCH net-next] Re: rib_trie / Fix inflate_threshold_root. Now=15 size=11 bits
From: Jarek Poplawski @ 2009-07-15 13:05 UTC (permalink / raw)
  To: Robert Olsson
  Cc: David Miller, Paweł Staszewski,
	Linux Network Development list, Robert Olsson,
	Jorge Boncompte [DTI2]
In-Reply-To: <19037.34959.327456.662899@robur.slu.se>

On Wed, Jul 15, 2009 at 09:43:11AM +0200, Robert Olsson wrote:
> 
> Jarek Poplawski writes:
> 
> 
> Looks good. Maybe we're getting close to some generic solution to take 
> a very optimistic approach wrt thresholds for root node and adjust to 
> settings without the warning. Or maybe now even remove warning totally
> with stata counter?

I guess, we could, but maybe let's wait a bit to make sure there is
nothing surprising?

> 
> Can we even consider some other different strategy for bumping up the root 
> node. 
> 
> We need all lookup performance we can get when we now try to route without 
> the route cache. And we probably need to evaluate the cost for the multiple 
> lookups again at least for LOCAL and MAIN when we talking routing well at 
> least straight-forward simple routing. (Semantic change)
> 
> I think I've got ~6.2 Gbit/s for simplex forwarding using traffic patterns 
> we see in/close to Internet core. This w/o route cache on our hi-end opterons
> with 8 CPU cores using niu and ixgbe. I'll test again and your patches when
> I'm back from vacation.
> 

Sure, I was mainly aiming at safe defaults (wrt. memory usage), but if
tests show there is a better strategy we should go for it.

Thanks,
Jarek P.

^ permalink raw reply

* [PATCH] net: nf_conntrack_alloc() should not use kmem_cache_zalloc()
From: Eric Dumazet @ 2009-07-15 12:28 UTC (permalink / raw)
  To: David Miller, kaber; +Cc: netdev, paulmck
In-Reply-To: <4A598BAB.6030400@gmail.com>

David, Patrick

Here is a fix for nf_conntrack, candidate for linux-2.6.31 and stable (linux-2.6.30)

Thank you

[PATCH] net: nf_conntrack_alloc() should not use kmem_cache_zalloc()

When a slab cache uses SLAB_DESTROY_BY_RCU, we must be careful when allocating
objects, since slab allocator could give a freed object still used by lockless
readers.

In particular, nf_conntrack RCU lookups rely on ct->tuplehash[xxx].hnnode.next
being always valid (ie containing a valid 'nulls' value, or a valid pointer to next
object in hash chain.)

kmem_cache_zalloc() setups object with NULL values, but a NULL value is not valid
for ct->tuplehash[xxx].hnnode.next.

Fix is to call kmem_cache_alloc() and do the zeroing ourself.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 7508f11..23feafa 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -561,17 +561,28 @@ struct nf_conn *nf_conntrack_alloc(struct net *net,
 		}
 	}
 
-	ct = kmem_cache_zalloc(nf_conntrack_cachep, gfp);
+	/*
+	 * Do not use kmem_cache_zalloc(), as this cache uses
+	 * SLAB_DESTROY_BY_RCU.
+	 */
+	ct = kmem_cache_alloc(nf_conntrack_cachep, gfp);
 	if (ct == NULL) {
 		pr_debug("nf_conntrack_alloc: Can't alloc conntrack.\n");
 		atomic_dec(&net->ct.count);
 		return ERR_PTR(-ENOMEM);
 	}
-
+	/*
+	 * Let ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.next
+	 * and ct->tuplehash[IP_CT_DIR_REPLY].hnnode.next unchanged.
+	 */
+	memset(&ct->tuplehash[IP_CT_DIR_MAX], 0,
+	       sizeof(*ct) - offsetof(struct nf_conn, tuplehash[IP_CT_DIR_MAX]));
 	spin_lock_init(&ct->lock);
 	atomic_set(&ct->ct_general.use, 1);
 	ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
+	ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
 	ct->tuplehash[IP_CT_DIR_REPLY].tuple = *repl;
+	ct->tuplehash[IP_CT_DIR_REPLY].hnnode.pprev = NULL;
 	/* Don't set timer yet: wait for confirmation */
 	setup_timer(&ct->timeout, death_by_timeout, (unsigned long)ct);
 #ifdef CONFIG_NET_NS


^ permalink raw reply related

* [PATCH] netfilter: ebtables: Use print_mac instead of own function
From: Tobias Klauser @ 2009-07-15 10:55 UTC (permalink / raw)
  To: bart.de.schuymer, shemminger, kaber, davem, ebtables-devel,
	netfilter-devel
  Cc: netdev, Tobias Klauser

ebt_log uses its own implementation of print_mac to print MAC addresses.
This patch converts it to use print_mac from linux/if_ether.h

Signed-off-by: Tobias Klauser <klto@zhaw.ch>
---
 net/bridge/netfilter/ebt_log.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/net/bridge/netfilter/ebt_log.c b/net/bridge/netfilter/ebt_log.c
index a94f3cc..70fd072 100644
--- a/net/bridge/netfilter/ebt_log.c
+++ b/net/bridge/netfilter/ebt_log.c
@@ -12,6 +12,7 @@
 #include <linux/ip.h>
 #include <linux/in.h>
 #include <linux/if_arp.h>
+#include <linux/if_ether.h>
 #include <linux/spinlock.h>
 #include <net/netfilter/nf_log.h>
 #include <linux/ipv6.h>
@@ -50,14 +51,6 @@ struct arppayload
 	unsigned char ip_dst[4];
 };
 
-static void print_MAC(const unsigned char *p)
-{
-	int i;
-
-	for (i = 0; i < ETH_ALEN; i++, p++)
-		printk("%02x%c", *p, i == ETH_ALEN - 1 ? ' ':':');
-}
-
 static void
 print_ports(const struct sk_buff *skb, uint8_t protocol, int offset)
 {
@@ -86,16 +79,16 @@ ebt_log_packet(u_int8_t pf, unsigned int hooknum,
    const char *prefix)
 {
 	unsigned int bitmask;
+	DECLARE_MAC_BUF(macbuf);
 
 	spin_lock_bh(&ebt_log_lock);
-	printk("<%c>%s IN=%s OUT=%s MAC source = ", '0' + loginfo->u.log.level,
-	       prefix, in ? in->name : "", out ? out->name : "");
-
-	print_MAC(eth_hdr(skb)->h_source);
-	printk("MAC dest = ");
-	print_MAC(eth_hdr(skb)->h_dest);
-
-	printk("proto = 0x%04x", ntohs(eth_hdr(skb)->h_proto));
+	printk("<%c>%s IN=%s OUT=%s MAC source = %s",
+	       '0' + loginfo->u.log.level, prefix,
+	       in ? in->name : "", out ? out->name : "",
+	       print_mac(macbuf, eth_hdr(skb)->h_source));
+	printk(" MAC dest = %s proto = 0x%04x",
+	       print_mac(macbuf, eth_hdr(skb)->h_dest),
+	       ntohs(eth_hdr(skb)->h_proto));
 
 	if (loginfo->type == NF_LOG_TYPE_LOG)
 		bitmask = loginfo->u.log.logflags;
@@ -171,11 +164,9 @@ ebt_log_packet(u_int8_t pf, unsigned int hooknum,
 				printk(" INCOMPLETE ARP payload");
 				goto out;
 			}
-			printk(" ARP MAC SRC=");
-			print_MAC(ap->mac_src);
+			printk(" ARP MAC SRC=%s", print_mac(macbuf, ap->mac_src));
 			printk(" ARP IP SRC=%pI4", ap->ip_src);
-			printk(" ARP MAC DST=");
-			print_MAC(ap->mac_dst);
+			printk(" ARP MAC DST=%s", print_mac(macbuf, ap->mac_dst));
 			printk(" ARP IP DST=%pI4", ap->ip_dst);
 		}
 	}
-- 
1.6.0.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