Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v6 1/8] Basic kernel memory functionality for the Memory Controller
From: KAMEZAWA Hiroyuki @ 2011-10-13  5:44 UTC (permalink / raw)
  To: Glauber Costa
  Cc: linux-kernel, paul, lizf, ebiederm, davem, gthelen, netdev,
	linux-mm, kirill, avagin, devel
In-Reply-To: <1318242268-2234-2-git-send-email-glommer@parallels.com>

On Mon, 10 Oct 2011 14:24:21 +0400
Glauber Costa <glommer@parallels.com> wrote:

> This patch lays down the foundation for the kernel memory component
> of the Memory Controller.
> 
> As of today, I am only laying down the following files:
> 
>  * memory.independent_kmem_limit
>  * memory.kmem.limit_in_bytes (currently ignored)
>  * memory.kmem.usage_in_bytes (always zero)
> 
> Signed-off-by: Glauber Costa <glommer@parallels.com>
> Reviewed-by: Kirill A. Shutemov <kirill@shutemov.name>
> CC: Paul Menage <paul@paulmenage.org>
> CC: Greg Thelen <gthelen@google.com>

Reviewed-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* linux-next: manual merge of the staging tree with the net tree
From: Stephen Rothwell @ 2011-10-13  5:43 UTC (permalink / raw)
  To: Greg KH
  Cc: linux-next, linux-kernel, Arend van Spriel, David Miller, netdev,
	Jiri Pirko, Eliad Peller, John W. Linville

[-- Attachment #1: Type: text/plain, Size: 491 bytes --]

Hi Greg,

Today's linux-next merge of the staging tree got a conflicts in
drivers/staging/brcm80211/brcmfmac/dhd_linux.c and
drivers/staging/brcm80211/brcmsmac/mac80211_if.c between commits from the
net-next tree and commit fc2d6e573be6 ("staging: brcm80211: remove
brcm80211 driver from the staging tree") from the staging tree.

The latter removed the files, so I did that
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Richard Cochran @ 2011-10-13  4:51 UTC (permalink / raw)
  To: Johannes Berg; +Cc: netdev, David Miller, Eric Dumazet
In-Reply-To: <1318447673.3933.47.camel@jlt3.sipsolutions.net>

On Wed, Oct 12, 2011 at 09:27:53PM +0200, Johannes Berg wrote:
> On Wed, 2011-10-12 at 20:36 +0200, Richard Cochran wrote:
> > The pair of functions,
> > 
> >  * skb_clone_tx_timestamp()
> >  * skb_complete_tx_timestamp()
...
> 
> This still needs a fix to the PHY driver right? It has a case that can
> kfree_skb() the skb instead of passing it back to
> complete_tx_timestamp().

Yes, right, will do.

> > -	if (!hwtstamps)
> > +	if (!hwtstamps) {
> > +		sock_put(sk);
> > +		kfree_skb(skb);
> >  		return;
> > +	}
> 
> Is that right w/o skb->sk = NULL?

I think it is okay. In clone/complete, we use skb->sk in a special
way, just to remember the socket. Within kfree_skb, only the
skb->destructor would possibly use skb->sk, and that function pointer
is NULL (after the clone in the Tx path).

> FWIW I just decided to do it the other way around in mac80211 -- keep
> the original SKB that's charged to the socket for the error queue, and
> use a clone to actually do the TX.

In this case, we must clone, I think. The Ethernet MAC driver will
call the clone_tx_timetstamp hook just before freeing the original
skb.

Thanks,
Richard

^ permalink raw reply

* Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY
From: David Daney @ 2011-10-13  4:45 UTC (permalink / raw)
  To: Grant Likely
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	afleming-Re5JQEeQqe8AvxtiuMwx3w, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20111013003129.GC14042-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>

On 10/12/2011 05:31 PM, Grant Likely wrote:
> On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote:
>> Add a driver and PHY_ID number for said device.  This is a 10Gig PHY
>> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
>> there is no autonegotiation.  All we do is report link state and send
>> interrupts when it changes.
>>
>> If the PHY has a device tree of_node associated with it, the
>> "broadcom,c45-reg-init" property is used to supply register
>> initialization values when config_init() is called.
>>
>> Signed-off-by: David Daney<david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> ---
>>
[...]
>> +The Broadcom BCM8706 is a 10G Ethernet PHY.  It has these bindings in
>> +addition to the standard PHY bindings.
>> +
>> +Compatible: Should contain "broadcom,bcm8706" and
>> +            "ethernet-phy-ieee802.3-c45"
>> +
>> +Optional Properties:
>> +
>> +- broadcom,c45-reg-init : one of more sets of 4 cells.  The first cell
>> +  is the device type, the second a register address, the third cell
>> +  contains a mask to be ANDed with the existing register value, and
>> +  the fourth cell is ORed with he result to yield the new register
>> +  value.
> ... a mask value of '0' should also guarantee that the driver does not do a read before the write.

The implementation does that, I will update the binding text to reflect 
this.

> What have we got so far in this regard for other phys and devices?

http://devicetree.org/Compatible%3Dmarvell,88e1149r

This is basically the same thing adapted for the page select register 
specific to Marvell PHYs.

>    I
> don't think it necessary to put 'c45' in the property name.  reg-init
> should be sufficient.

10M/100M/1G PHYs from different manufacturers and even within a single 
manufacturer have a wide variety of ways to multiplex many registers 
into the 5 bit addressing scheme allowed by clause 22.  The Marvell 
scheme already implemented doesn't work for Broadcom.

For clause 45, there are more address bits...

>    I'd like to hear from others if it would be
> valuable to have a 'reg-init-sequence' property of the above format.

A clause 45 specific property might work for *all* 10G PHYs, the same 
cannot be said for clause 22, hence my idea to put 'c45' in the name

> What does the device type cell indicate?  Wouldn't the driver
> naturally have the device id from the address of the cell?
>

There are three portions to a clause 45 address:

phy_id:  Denoted by the "reg" property is a 5-bit value that identifies 
a particular PHY on the MDIO bus.

device id: Really a sub-device within a given PHY, another 5-bit value 
contained in the first cell of the proposed register init sequence.  
Clause 45 defines several different standard device ids.

register id: a 16-bit address that identifies a particular 16-bit 
register within the 'device' (or sub-device if you will.

Does that answer your question?

In theory we could compose the 5-bit device id and 16-bit register 
address into a single 32-bit cell in the init sequence property, but I 
chose to have them separate.

>> +static int __init bcm8706_init(void)
>> +{
>> +	int ret;
>> +
>> +	ret = phy_driver_register(&bcm8706_driver);
>> +
>> +	return ret;
>> +}
>> +module_init(bcm8706_init);
> or simply:
> static int __init bcm8706_init(void)
> {
> 	return phy_driver_register(&bcm8706_driver);
> }
> module_init(bcm8706_init);
>

Yes, I will make that change.

David Daney

^ permalink raw reply

* Re: [PATCH 1/3] netdev/phy: Handle IEEE802.3 clause 45 Ethernet PHYs
From: Grant Likely @ 2011-10-13  0:21 UTC (permalink / raw)
  To: David Daney; +Cc: devicetree-discuss, linux-kernel, netdev, davem, afleming
In-Reply-To: <1318442783-29058-2-git-send-email-david.daney@cavium.com>

On Wed, Oct 12, 2011 at 11:06:21AM -0700, David Daney wrote:
> The IEEE802.3 clause 45 MDIO bus protocol allows for directly
> addressing PHY registers using a 21 bit address, and is used by many
> 10G Ethernet PHYS.  Already existing is the ability of MDIO bus
> drivers to use clause 45, with the MII_ADDR_C45 flag.  Here we add
> some support in the PHY and device tree infrastructure to use these
> PHYs.
> 
> Normally the MII_ADDR_C45 flag is ORed with the register address to
> indicate a clause 45 transaction.  Here we also use this flag in the
> *device* address passed to get_phy_id() and get_phy_device() to
> indicate that probing should be done with clause 45 transactions.  If
> a PHY is successfully probed with MII_ADDR_C45, the new struct
> phy_device is_c45 flag is set for the PHY.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

Minor comment below, but otherwise,

Acked-by: Grant Likely <grant.likely@secretlab.ca>

> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index e4c3844..0a25e2c 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -246,6 +246,7 @@ struct sk_buff;
>   * phy_id: UID for this device found during discovery
>   * state: state of the PHY for management purposes
>   * dev_flags: Device-specific flags used by the PHY driver.
> + * is_c45:  Set to 1 if this phy uses clause 45 addressing.

"Set to true if ..."

>   * addr: Bus address of PHY
>   * link_timeout: The number of timer firings to wait before the
>   * giving up on the current attempt at acquiring a link
> @@ -283,6 +284,8 @@ struct phy_device {
>  
>  	u32 dev_flags;
>  
> +	unsigned int is_c45:1;
> +

bool

>  	phy_interface_t interface;
>  
>  	/* Bus address of the PHY (0-31) */
> -- 
> 1.7.2.3
> 

^ permalink raw reply

* __kfree_skb eventually calls kfree_skb violating dropwatch assumption
From: Suresh, Charles @ 2011-10-13  2:25 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: nhorman@redhat.com

kfree_skb can be called from __kfree_skb through the following call chain :

kfree_skb  <- skb_drop_list <- skb_drop_fraglist <- skb_release_data <-  skb_release_all <- __kfree_skb (on the 2.38.4 kernel).

This violates the assumption in the dropwatch tool that discarded packets go through the kfree_skb path and all others must go through the consume_skb path (thus resulting in the over-counting of discarded packets in dropwatch).

Neil Horman the author of dropwatch suggested that this could be fixed by skb_drop_list calling consume_skb instead of kfree_skb.

Charles

^ permalink raw reply

* [PATCH net -v2] [BUGFIX] bonding: use local function pointer of bond->recv_probe in bond_handle_frame
From: Mitsuo Hayasaka @ 2011-10-13  2:04 UTC (permalink / raw)
  To: Jay Vosburgh, Andy Gospodarek
  Cc: netdev, linux-kernel, yrl.pp-manager.tt, Mitsuo Hayasaka,
	Jay Vosburgh, Andy Gospodarek, Eric Dumazet, WANG Cong

The bond->recv_probe is called in bond_handle_frame() when
a packet is received, but bond_close() sets it to NULL. So,
a panic occurs when both functions work in parallel.

Why this happen:
After null pointer check of bond->recv_probe, an sk_buff is
duplicated and bond->recv_probe is called in bond_handle_frame.
So, a panic occurs when bond_close() is called between the
check and call of bond->recv_probe.

Patch:
This patch uses a local function pointer of bond->recv_probe
in bond_handle_frame(). So, it can avoid the null pointer
dereference.


Signed-off-by: Mitsuo Hayasaka <mitsuo.hayasaka.hu@hitachi.com>
Cc: Jay Vosburgh <fubar@us.ibm.com>
Cc: Andy Gospodarek <andy@greyhouse.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: WANG Cong <xiyou.wangcong@gmail.com>
---

 drivers/net/bonding/bond_main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 6d79b78..de3d351 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1435,6 +1435,8 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 	struct sk_buff *skb = *pskb;
 	struct slave *slave;
 	struct bonding *bond;
+	void (*recv_probe)(struct sk_buff *, struct bonding *,
+				struct slave *);
 
 	skb = skb_share_check(skb, GFP_ATOMIC);
 	if (unlikely(!skb))
@@ -1448,11 +1450,12 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
 	if (bond->params.arp_interval)
 		slave->dev->last_rx = jiffies;
 
-	if (bond->recv_probe) {
+	recv_probe = ACCESS_ONCE(bond->recv_probe);
+	if (recv_probe) {
 		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
 
 		if (likely(nskb)) {
-			bond->recv_probe(nskb, bond, slave);
+			recv_probe(nskb, bond, slave);
 			dev_kfree_skb(nskb);
 		}
 	}

^ permalink raw reply related

* Re: [PATCH 3/3] netdev/phy: Add driver for Broadcom BCM8706 10G Ethernet PHY
From: Grant Likely @ 2011-10-13  0:31 UTC (permalink / raw)
  To: David Daney; +Cc: devicetree-discuss, linux-kernel, netdev, davem, afleming
In-Reply-To: <1318442783-29058-4-git-send-email-david.daney@cavium.com>

On Wed, Oct 12, 2011 at 11:06:23AM -0700, David Daney wrote:
> Add a driver and PHY_ID number for said device.  This is a 10Gig PHY
> which uses MII_ADDR_C45 addressing, it is always 10G full duplex, so
> there is no autonegotiation.  All we do is report link state and send
> interrupts when it changes.
> 
> If the PHY has a device tree of_node associated with it, the
> "broadcom,c45-reg-init" property is used to supply register
> initialization values when config_init() is called.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  .../devicetree/bindings/net/broadcom-bcm8706.txt   |   28 +++
>  drivers/net/phy/Kconfig                            |    5 +
>  drivers/net/phy/Makefile                           |    1 +
>  drivers/net/phy/bcm8706.c                          |  212 ++++++++++++++++++++
>  include/linux/brcmphy.h                            |    1 +
>  5 files changed, 247 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
>  create mode 100644 drivers/net/phy/bcm8706.c
> 
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> new file mode 100644
> index 0000000..d58bea9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bcm8706.txt
> @@ -0,0 +1,28 @@
> +The Broadcom BCM8706 is a 10G Ethernet PHY.  It has these bindings in
> +addition to the standard PHY bindings.
> +
> +Compatible: Should contain "broadcom,bcm8706" and
> +            "ethernet-phy-ieee802.3-c45"
> +
> +Optional Properties:
> +
> +- broadcom,c45-reg-init : one of more sets of 4 cells.  The first cell
> +  is the device type, the second a register address, the third cell
> +  contains a mask to be ANDed with the existing register value, and
> +  the fourth cell is ORed with he result to yield the new register
> +  value.

... a mask value of '0' should also guarantee that the driver does not do a read before the write.

What have we got so far in this regard for other phys and devices?  I
don't think it necessary to put 'c45' in the property name.  reg-init
should be sufficient.  I'd like to hear from others if it would be
valuable to have a 'reg-init-sequence' property of the above format.

What does the device type cell indicate?  Wouldn't the driver
naturally have the device id from the address of the cell?

> +static int __init bcm8706_init(void)
> +{
> +	int ret;
> +
> +	ret = phy_driver_register(&bcm8706_driver);
> +
> +	return ret;
> +}
> +module_init(bcm8706_init);

or simply:
static int __init bcm8706_init(void)
{
	return phy_driver_register(&bcm8706_driver);
}
module_init(bcm8706_init);

Otherwise the driver code seems to be fine.

g.

^ permalink raw reply

* Re: [PATCH 2/3] netdev/phy/of: Handle IEEE802.3 clause 45 Ethernet PHYs in of_mdiobus_register()
From: Grant Likely @ 2011-10-13  0:23 UTC (permalink / raw)
  To: David Daney; +Cc: devicetree-discuss, linux-kernel, netdev, davem, afleming
In-Reply-To: <1318442783-29058-3-git-send-email-david.daney@cavium.com>

On Wed, Oct 12, 2011 at 11:06:22AM -0700, David Daney wrote:
> Define two new "compatible" values for Ethernet
> PHYs. "ethernet-phy-ieee802.3-c22" and "ethernet-phy-ieee802.3-c45"
> are used to indicate a PHY uses the corresponding protocol.
> 
> If a PHY is "compatible" with "ethernet-phy-ieee802.3-c45", we
> indicate this so that get_phy_device() can properly probe the device.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>

I'm okay with this binding.  I'd like to get opinions from other
developers before it is committed to though.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

g.

> ---
>  Documentation/devicetree/bindings/net/phy.txt |   12 +++++++++++-
>  drivers/of/of_mdio.c                          |    4 ++++
>  2 files changed, 15 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
> index bb8c742..de9cb32 100644
> --- a/Documentation/devicetree/bindings/net/phy.txt
> +++ b/Documentation/devicetree/bindings/net/phy.txt
> @@ -16,8 +16,18 @@ Required properties:
>  
>  Example:
>  
> +Optional Properties:
> +
> +- compatible: Compatible list, may contain "ethernet-phy-ieee802.3-c22" or
> +              "ethernet-phy-ieee802.3-c45" for PHYs that implement
> +              IEEE802.3 clause 22 or IEEE802.3 clause 45
> +              specifications.  If neither of these are specified, the
> +              default is to assume clause 22.  The compatible list may
> +              also contain other elements.
> +
>  ethernet-phy@0 {
> -	linux,phandle = <2452000>
> +	compatible = "ethernet-phy-ieee802.3-c22";
> +	linux,phandle = <2452000>;
>  	interrupt-parent = <40000>;
>  	interrupts = <35 1>;
>  	reg = <0>;
> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
> index 7c28e8c..f837a7f 100644
> --- a/drivers/of/of_mdio.c
> +++ b/drivers/of/of_mdio.c
> @@ -79,6 +79,10 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
>  				mdio->irq[addr] = PHY_POLL;
>  		}
>  
> +		if (of_device_is_compatible(child,
> +					    "ethernet-phy-ieee802.3-c45"))
> +			addr |= MII_ADDR_C45;
> +
>  		phy = get_phy_device(mdio, addr);
>  		if (!phy || IS_ERR(phy)) {
>  			dev_err(&mdio->dev, "error probing PHY at address %i\n",
> -- 
> 1.7.2.3
> 

^ permalink raw reply

* Re: [patch net-2.6] tg3: negate USE_PHYLIB flag check
From: Matt Carlson @ 2011-10-12 23:55 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev@vger.kernel.org, davem@davemloft.net,
	eric.dumazet@gmail.com, Matthew Carlson, Michael Chan
In-Reply-To: <1318410041-2476-1-git-send-email-jpirko@redhat.com>

On Wed, Oct 12, 2011 at 02:00:41AM -0700, Jiri Pirko wrote:
> USE_PHYLIB flag in tg3_remove_one() is being checked incorrectly. This
> results tg3_phy_fini->phy_disconnect is never called and when tg3 module
> is removed.
> 
> In my case this resulted in panics in phy_state_machine calling function
> phydev->adjust_link.
> 
> So correct this check.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Introduced by commit 63c3a66fe6c827a731dcbdee181158b295626f83, entitled
"tg3: Convert u32 flag,flg2,flg3 uses to bitmap".

Acked-by: Matt Carlson <mcarlson@broadcom.com>

> ---
>  drivers/net/tg3.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index 4a1374d..c11a2b8 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -15577,7 +15577,7 @@ static void __devexit tg3_remove_one(struct pci_dev *pdev)
>  
>  		cancel_work_sync(&tp->reset_task);
>  
> -		if (!tg3_flag(tp, USE_PHYLIB)) {
> +		if (tg3_flag(tp, USE_PHYLIB)) {
>  			tg3_phy_fini(tp);
>  			tg3_mdio_fini(tp);
>  		}
> -- 
> 1.7.6.2
> 
> 

^ permalink raw reply

* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: David Miller @ 2011-10-12 23:10 UTC (permalink / raw)
  To: eric.dumazet
  Cc: muralira, kuznet, jmorris, yoshfuji, kaber, linux-kernel, netdev
In-Reply-To: <1318446934.2644.0.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 12 Oct 2011 21:15:34 +0200

> Le mercredi 12 octobre 2011 à 12:00 -0700, Muraliraja Muniraju a écrit :
>> From: Murali Raja <muralira@google.com>
>> 
>> This patch exposes the tos value for the TCP sockets when the TOS flag
>> is requested in the ext_flags for the inet_diag request. This would mainly be
>> used to expose TOS values for both for TCP and UDP sockets. Currently it is
>> supported for TCP. When netlink support for UDP would be added the support
>> to expose the TOS values would alse be done. For IPV4 tos value is exposed
>> and for IPV6 tclass value is exposed.
>> 
>> Signed-off-by: Murali Raja <muralira@google.com>
>> ---
> 
> Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH net-next 0/5] intel: Logging cleanups
From: Jeff Kirsher @ 2011-10-12 22:56 UTC (permalink / raw)
  To: Joe Perches
  Cc: netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net,
	linux-kernel@vger.kernel.org
In-Reply-To: <cover.1318436085.git.joe@perches.com>

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

On Wed, 2011-10-12 at 09:18 -0700, Joe Perches wrote:
> Just some conversions of printks to pr_<level>
> and some trivial defect corrections.
> 
> Joe Perches (5):
>   e1000e: Convert printks to pr_<level>
>   ixgbevf: Convert printks to pr_<level>
>   igbvf: Convert printks to pr_<level>
>   igb: Convert printks to pr_<level>
>   igb: Convert bare printk to pr_notice
> 
>  drivers/net/ethernet/intel/e1000e/netdev.c        |  232 +++++++++------------
>  drivers/net/ethernet/intel/igb/e1000_82575.c      |    5 +-
>  drivers/net/ethernet/intel/igb/igb_main.c         |  155 +++++++--------
>  drivers/net/ethernet/intel/igbvf/netdev.c         |   14 +-
>  drivers/net/ethernet/intel/ixgbevf/ethtool.c      |    6 +-
>  drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   26 ++--
>  6 files changed, 201 insertions(+), 237 deletions(-)
> 

Thanks Joe.  I will add these to my queue of patches.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* RE: [net-next 10/12] igb: move DMA Coalescing feature code into separate function.
From: Wyborny, Carolyn @ 2011-10-12 22:21 UTC (permalink / raw)
  To: David Miller, Kirsher, Jeffrey T
  Cc: Sowles, Jacob R, netdev@vger.kernel.org, gospo@redhat.com,
	sassmann@redhat.com
In-Reply-To: <20111012.164834.2208432778408483576.davem@davemloft.net>



>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>On Behalf Of David Miller
>Sent: Wednesday, October 12, 2011 1:49 PM
>To: Kirsher, Jeffrey T
>Cc: Sowles, Jacob R; netdev@vger.kernel.org; gospo@redhat.com;
>sassmann@redhat.com
>Subject: Re: [net-next 10/12] igb: move DMA Coalescing feature code into
>separate function.
>
>From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>Date: Tue, 11 Oct 2011 20:38:26 -0700
>
>> From: Jacob Sowles <jacob.r.sowles@intel.com>
>>
>> DMA Coalescing code needs to be executed during reset, if enabled.
>Moved
>> code to separate function and added a call to it in igb_reset.
>>
>> Signed-off-by: Jacob Sowles <jacob.r.sowles@intel.com>
>> Tested-by:  Aaron Brown <aaron.f.brown@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>
>Something is wrong here, this patch just adds the new function but no
>actual callers.
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yes, you are correct.  We had a problem in our process and this patch got inadvertently changed during it, removing a substantial part of it, including the call to the new function.  I'm on it and will be submitting a replacement as soon as possible.

Thanks,

Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation

^ permalink raw reply

* Re: [patch] cipso: remove an unneeded NULL check in cipso_v4_doi_add()
From: Paul Moore @ 2011-10-12 21:28 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, kernel-janitors
In-Reply-To: <20111011215549.GC30887@longonot.mountain>

On Wednesday, October 12, 2011 12:55:49 AM Dan Carpenter wrote:
> On Tue, Oct 11, 2011 at 05:20:11PM -0400, Paul Moore wrote:
> > > -       if (doi_def == NULL || doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
> > > +       if (doi_def->doi == CIPSO_V4_DOI_UNKNOWN)
> > >                goto doi_add_return;
> > >        for (iter = 0; iter < CIPSO_V4_TAG_MAXCNT; iter++) {
> > >                switch (doi_def->tags[iter]) {
> > 
> > I'd prefer to keep the NULL check in there as it does afford a little
> > bit of extra safety and this is management code after all, not
> > per-packet processing code, so the extra check should have no
> > observable performance impact.
> 
> The dereferences on the lines before mean we would Oops before
> reaching the check ...

Thanks for pointing that out, I missed that when looking at your patch.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH] bonding: L2L3 xmit doesn't support IPv6
From: John Eaglesham @ 2011-10-12 21:15 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Yinglin Sun, Andy Gospodarek, Jay Vosburgh,
	netdev@vger.kernel.org, linux
In-Reply-To: <1318392395.3686.12.camel@edumazet-laptop>

Eric Dumazet wrote:
> Le mardi 11 octobre 2011 à 20:39 -0700, Yinglin Sun a écrit :
>> On Tue, Oct 11, 2011 at 7:51 PM, Andy Gospodarek <andy@greyhouse.net> wrote:
>>>        if (skb->protocol == htons(ETH_P_IP)) {
>>> +               struct iphdr *iph = ip_hdr(skb);
>>> +               __be16 *layer4hdr = (__be16 *)((u32 *)iph + iph->ihl);
>>>                if (!ip_is_fragment(iph) &&
>>>                    (iph->protocol == IPPROTO_TCP ||
>>>                     iph->protocol == IPPROTO_UDP)) {
>>> @@ -3398,7 +3407,18 @@ static int bond_xmit_hash_policy_l34(struct sk_buff *skb, int count)
>>>                }
>>>                return (layer4_xor ^
>>>                        ((ntohl(iph->saddr ^ iph->daddr)) & 0xffff)) % count;
>>> -
>>> +       } else if (skb->protocol == htons(ETH_P_IPV6)) {
>>> +               struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>>> +               __be16 *layer4hdrv6 = (__be16 *)((u8 *)ipv6h + sizeof(*ipv6h));
>>> +               if (ipv6h->nexthdr == IPPROTO_TCP || ipv6h->nexthdr == IPPROTO_UDP) {
>> Does this work if this is a fragmentation packet? and if
>> ipv6h->nexthdr is not IPPROTO_TCP/IPPROTO_UDP, it doesn't mean this is
>> not TCP/UDP packet. We need to go through the extension header chain
>> and look at the last one. It's likely there are some other extension
>> headers before L4 header.
>>
> 
> Its a best effort.
> 
> If first header is not IPPROTO_TCP/UDP, I am not sure its wise to spend
> time in hope to find layer4 info (missing anyway in fragments)
> 
> By the way I see no bound checking on SKB head : malicious packet could
> make bond_xmit_hash_policy_l34() access unitialized memory.
> 
> We have same 'fastpath' in __skb_get_rxhash()
> 
> 

Thanks for keeping me in the loop on this.

I caught the bounds checking bug and added checks to code I haven't 
submitted in yet (I still need to test that it works before I submit a 
patch).

I don't like the idea of sticking a loop in this part of the code, 
especially one traversing a list of length controlled by outside users 
with no guarantee it is properly formed. I ran some tests gathering 
traffic and I never saw any packets with headers between the IPv6 and 
TCP or UDP, so at least in the real world right now this code should 
behave as expected for the vast majority of traffic. Things might change 
in the future, but if this code is clear/clean/simple and solves the 
problem today, I'd be happier with that than trying to parse a full IPv6 
header.

I'll test and post my revised patch (including bounds checking and 
documentation updates) in a day or so.

John

^ permalink raw reply

* [PATCH] [TRIVIAL] isdn: hisax: Fix typo 'HISAX_DE_AOC'
From: Paul Bolle @ 2011-10-12 21:19 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-kernel, Karsten Keil, netdev

That should probably be 'CONFIG_DE_AOC'.

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
This is just the most obvious way to reconcile an unused Kconfig symbol
(DE_AOC) and an unknown macro (HISAX_DE_AOC). But it's basically just
educated guesswork. Entirely untested too. Added maintainer and netdev,
since this might be stretching the definition of a trivial patch.

 drivers/isdn/hisax/l3dss1.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/isdn/hisax/l3dss1.c b/drivers/isdn/hisax/l3dss1.c
index b0d9ab1..6a8acf6 100644
--- a/drivers/isdn/hisax/l3dss1.c
+++ b/drivers/isdn/hisax/l3dss1.c
@@ -353,7 +353,7 @@ l3dss1_parse_facility(struct PStack *st, struct l3_process *pc,
 			         { l3dss1_dummy_invoke(st, cr, id, ident, p, nlen);
                                    return;
                                  } 
-#ifdef HISAX_DE_AOC
+#ifdef CONFIG_DE_AOC
 			{
 
 #define FOO1(s,a,b) \
@@ -422,9 +422,9 @@ l3dss1_parse_facility(struct PStack *st, struct l3_process *pc,
 #undef FOO1
 
 			}
-#else  /* not HISAX_DE_AOC */
+#else  /* not CONFIG_DE_AOC */
                         l3_debug(st, "invoke break");
-#endif /* not HISAX_DE_AOC */
+#endif /* not CONFIG_DE_AOC */
 			break;
 		case 2:	/* return result */
 			 /* if no process available handle separately */ 
-- 
1.7.4.4

^ permalink raw reply related

* Re: [PATCH 2/4] ipv4: Update pmtu informations on inetpeer only for output routes
From: David Miller @ 2011-10-12 21:08 UTC (permalink / raw)
  To: steffen.klassert; +Cc: netdev
In-Reply-To: <20111011111027.GE1830@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 11 Oct 2011 13:10:27 +0200

> @@ -1817,9 +1819,14 @@ static void rt_init_metrics(struct rtable *rt, const struct flowi4 *fl4,
>  		if (inet_metrics_new(peer))
>  			memcpy(peer->metrics, fi->fib_metrics,
>  			       sizeof(u32) * RTAX_MAX);
> -		dst_init_metrics(&rt->dst, peer->metrics, false);
>  
> -		check_peer_pmtu(&rt->dst, peer);
> +		dst_init_metrics(dst, peer->metrics, false);
> +		check_peer_pmtu(dst, peer);
> +
> +		if (rt_is_input_route(rt))
> +			dst_metric_set(dst, RTAX_MTU,
> +				       dst->ops->default_mtu(dst));
> +

You really can't do this, it's going to kill all of the memory savings from
storing metrics in the inetpeer cache.

Every input route is going to have it's metrics COW'd with this change.

The whole idea is to use defaults as heavily as possible, and that's
the entire reason why the dst->ops->default_mtu() method exists, so
that we can just leave the values alone and have read-only copies %99
of the time.

Please rearrange your fix so that these goals are still achieved.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/4] ipv4: Fix pmtu propagating
From: David Miller @ 2011-10-12 21:02 UTC (permalink / raw)
  To: steffen.klassert; +Cc: netdev
In-Reply-To: <20111011110922.GD1830@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Tue, 11 Oct 2011 13:09:22 +0200

> Since commit 2c8cec5c (ipv4: Cache learned PMTU information in inetpeer)
> we cache the learned pmtu informations in inetpeer and propagate these
> informations with the dst_ops->check() functions. However, these functions
> might not be called for udp and raw packets. As a consequence, we don't
> use the learned pmtu informations in these cases. With this patch we
> call dst_check() from ip_setup_cork() to propagate the pmtu informations.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>

This dst_check() call will only do something if dst->obsolete is non-zero.

If dst->obsolete can be set in these circumstances, that's a bug.  The
caller is responsible for providing either a freshly looked up route
or a cached route which has had dst_check() or sk_dst_check() invoked
upon it.

I am pretty sure these rules are followed by the current code.

Again, there are only two scenerios:

1) 'rt' is just looked up by caller (f.e. udp_sendmsg() in rt == NULL case),
   here dst->obsolete is very unlikely to be non-zero.

2) Connected case, and we use cached route from the socket, but here
   we'll use sk_dst_check() to validate the route.  sk_dst_check()
   makes the necessary dst->ops->check() call if dst->obsolete is
   non-zero, and in fact that is it's one and only job.

So I cannot see a case where your new check can be necessary.

^ permalink raw reply

* Re: [net-next 10/12] igb: move DMA Coalescing feature code into separate function.
From: David Miller @ 2011-10-12 20:48 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: jacob.r.sowles, netdev, gospo, sassmann
In-Reply-To: <1318390708-12232-11-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 11 Oct 2011 20:38:26 -0700

> From: Jacob Sowles <jacob.r.sowles@intel.com>
> 
> DMA Coalescing code needs to be executed during reset, if enabled.  Moved
> code to separate function and added a call to it in igb_reset.
> 
> Signed-off-by: Jacob Sowles <jacob.r.sowles@intel.com>
> Tested-by:  Aaron Brown <aaron.f.brown@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Something is wrong here, this patch just adds the new function but no
actual callers.

^ permalink raw reply

* Re: [net-next 2/5] stmmac: allow mtu bigger than 1500 in case of normal desc (V2).
From: Eric Dumazet @ 2011-10-12 19:58 UTC (permalink / raw)
  To: Giuseppe CAVALLARO; +Cc: netdev, davem, Deepak SIKRI
In-Reply-To: <1318426688-9419-3-git-send-email-peppe.cavallaro@st.com>

Le mercredi 12 octobre 2011 à 15:38 +0200, Giuseppe CAVALLARO a écrit :
> This patch allows to set the mtu bigger than 1500
> in case of normal descriptors.
> This is helping some SPEAr customers.
> 
> Signed-off-by: Deepak SIKRI <deepak.sikri@st.com>
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index ba7af2c..de3e536 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1357,17 +1357,17 @@ static void stmmac_set_rx_mode(struct net_device *dev)
>  static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
>  {
>  	struct stmmac_priv *priv = netdev_priv(dev);
> -	int max_mtu;
> +	int max_mtu = ETH_DATA_LEN;

Why are you setting max_mtu to ETH_DATA_LEN here ?

>  
>  	if (netif_running(dev)) {
>  		pr_err("%s: must be stopped to change its MTU\n", dev->name);
>  		return -EBUSY;
>  	}
>  
> -	if (priv->plat->has_gmac)
> +	if (priv->plat->enh_desc)
>  		max_mtu = JUMBO_LEN;
>  	else
> -		max_mtu = ETH_DATA_LEN;
> +		max_mtu = BUF_SIZE_4KiB;

Since later you init to completely different values...

^ permalink raw reply

* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Eric Dumazet @ 2011-10-12 19:52 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Richard Cochran, netdev, David Miller
In-Reply-To: <1318447673.3933.47.camel@jlt3.sipsolutions.net>

Le mercredi 12 octobre 2011 à 21:27 +0200, Johannes Berg a écrit :
> On Wed, 2011-10-12 at 20:36 +0200, Richard Cochran wrote:
> > The pair of functions,
> > 
> >  * skb_clone_tx_timestamp()
> >  * skb_complete_tx_timestamp()
> > 
> > were designed to allow timestamping in PHY devices. The first
> > function, called during the MAC driver's hard_xmit method, identifies
> > PTP protocol packets, clones them, and gives them to the PHY device
> > driver. The PHY driver may hold onto the packet and deliver it at a
> > later time using the second function, which adds the packet to the
> > socket's error queue.
> > 
> > As pointed out by Johannes, nothing prevents the socket from
> > disappearing while the cloned packet is sitting in the PHY driver
> > awaiting a timestamp. This patch fixes the issue by taking a reference
> > on the socket for each such packet. In addition, the comments
> > regarding the usage of these function are expanded to highlight the
> > rule that PHY drivers must use skb_complete_tx_timestamp() to release
> > the packet, in order to release the socket reference, too.
> 
> This still needs a fix to the PHY driver right? It has a case that can
> kfree_skb() the skb instead of passing it back to
> complete_tx_timestamp().
> 
> > -	if (!hwtstamps)
> > +	if (!hwtstamps) {
> > +		sock_put(sk);
> > +		kfree_skb(skb);
> >  		return;
> > +	}
> 
> Is that right w/o skb->sk = NULL?
> 
> 
> The other thing I was wondering -- what if we just set truesize to 1 (we
> don't have any truesize checks) and account the skb to the socket
> normally. Not really a good way either though.
> 

Changing truesize is not allowed here see below.

> FWIW I just decided to do it the other way around in mac80211 -- keep
> the original SKB that's charged to the socket for the error queue, and
> use a clone to actually do the TX.

Hmm, please take a look at IFF_SKB_TX_SHARING stuff, added in commit
d88733150 (net: add IFF_SKB_TX_SHARED flag to priv_flags)

^ permalink raw reply

* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Johannes Berg @ 2011-10-12 19:27 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David Miller, Eric Dumazet
In-Reply-To: <56185ca8a7dc0223031ca0f0996302cac1b497eb.1318444117.git.richard.cochran@omicron.at>

On Wed, 2011-10-12 at 20:36 +0200, Richard Cochran wrote:
> The pair of functions,
> 
>  * skb_clone_tx_timestamp()
>  * skb_complete_tx_timestamp()
> 
> were designed to allow timestamping in PHY devices. The first
> function, called during the MAC driver's hard_xmit method, identifies
> PTP protocol packets, clones them, and gives them to the PHY device
> driver. The PHY driver may hold onto the packet and deliver it at a
> later time using the second function, which adds the packet to the
> socket's error queue.
> 
> As pointed out by Johannes, nothing prevents the socket from
> disappearing while the cloned packet is sitting in the PHY driver
> awaiting a timestamp. This patch fixes the issue by taking a reference
> on the socket for each such packet. In addition, the comments
> regarding the usage of these function are expanded to highlight the
> rule that PHY drivers must use skb_complete_tx_timestamp() to release
> the packet, in order to release the socket reference, too.

This still needs a fix to the PHY driver right? It has a case that can
kfree_skb() the skb instead of passing it back to
complete_tx_timestamp().

> -	if (!hwtstamps)
> +	if (!hwtstamps) {
> +		sock_put(sk);
> +		kfree_skb(skb);
>  		return;
> +	}

Is that right w/o skb->sk = NULL?


The other thing I was wondering -- what if we just set truesize to 1 (we
don't have any truesize checks) and account the skb to the socket
normally. Not really a good way either though.

FWIW I just decided to do it the other way around in mac80211 -- keep
the original SKB that's charged to the socket for the error queue, and
use a clone to actually do the TX.

johannes

^ permalink raw reply

* Re: [PATCH 1/1] net: hold sock reference while processing tx timestamps
From: Eric Dumazet @ 2011-10-12 19:25 UTC (permalink / raw)
  To: Richard Cochran; +Cc: netdev, David Miller, Johannes Berg, stable
In-Reply-To: <56185ca8a7dc0223031ca0f0996302cac1b497eb.1318444117.git.richard.cochran@omicron.at>

Le mercredi 12 octobre 2011 à 20:36 +0200, Richard Cochran a écrit :
> The pair of functions,
> 
>  * skb_clone_tx_timestamp()
>  * skb_complete_tx_timestamp()
> 
> were designed to allow timestamping in PHY devices. The first
> function, called during the MAC driver's hard_xmit method, identifies
> PTP protocol packets, clones them, and gives them to the PHY device
> driver. The PHY driver may hold onto the packet and deliver it at a
> later time using the second function, which adds the packet to the
> socket's error queue.
> 
> As pointed out by Johannes, nothing prevents the socket from
> disappearing while the cloned packet is sitting in the PHY driver
> awaiting a timestamp. This patch fixes the issue by taking a reference
> on the socket for each such packet. In addition, the comments
> regarding the usage of these function are expanded to highlight the
> rule that PHY drivers must use skb_complete_tx_timestamp() to release
> the packet, in order to release the socket reference, too.
> 
> These functions first appeared in v2.6.36.
> 
> Reported-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> Cc: <stable@kernel.org>
> ---

Seems fine to me, thanks !

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: Eric Dumazet @ 2011-10-12 19:15 UTC (permalink / raw)
  To: Muraliraja Muniraju
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel, netdev
In-Reply-To: <1318446035-10267-1-git-send-email-muralira@google.com>

Le mercredi 12 octobre 2011 à 12:00 -0700, Muraliraja Muniraju a écrit :
> From: Murali Raja <muralira@google.com>
> 
> This patch exposes the tos value for the TCP sockets when the TOS flag
> is requested in the ext_flags for the inet_diag request. This would mainly be
> used to expose TOS values for both for TCP and UDP sockets. Currently it is
> supported for TCP. When netlink support for UDP would be added the support
> to expose the TOS values would alse be done. For IPV4 tos value is exposed
> and for IPV6 tclass value is exposed.
> 
> Signed-off-by: Murali Raja <muralira@google.com>
> ---

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>

^ permalink raw reply

* Re: [PATCH v4] net-netlink: Add a new attribute to expose TOS values via netlink
From: Stephen Hemminger @ 2011-10-12 19:08 UTC (permalink / raw)
  To: Muraliraja Muniraju
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, linux-kernel, netdev
In-Reply-To: <1318446035-10267-1-git-send-email-muralira@google.com>

On Wed, 12 Oct 2011 12:00:35 -0700
Muraliraja Muniraju <muralira@google.com> wrote:

> From: Murali Raja <muralira@google.com>
> 
> This patch exposes the tos value for the TCP sockets when the TOS flag
> is requested in the ext_flags for the inet_diag request. This would mainly be
> used to expose TOS values for both for TCP and UDP sockets. Currently it is
> supported for TCP. When netlink support for UDP would be added the support
> to expose the TOS values would alse be done. For IPV4 tos value is exposed
> and for IPV6 tclass value is exposed.
> 
> Signed-off-by: Murali Raja <muralira@google.com>
> ---
> Changelog since v3:
> - Removed the tos structure from the inet_diag.h
> 
> Changelog since v2:
> - Added support for IPv6 and used better API.
> 
> Changelog since v3:
> - Removed reserved fields.


After this is accepted; it is trivial to add support to ss command.

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

^ permalink raw reply


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