Netdev List
 help / color / mirror / Atom feed
* [PATCH 04/52] net: pcnet32: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-11  7:25 UTC (permalink / raw)
  To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han', 'Don Fry'
In-Reply-To: <004a01ceaebf$43f920f0$cbeb62d0$%han@samsung.com>

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/net/ethernet/amd/pcnet32.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/pcnet32.c b/drivers/net/ethernet/amd/pcnet32.c
index 2d8e288..40b92e3 100644
--- a/drivers/net/ethernet/amd/pcnet32.c
+++ b/drivers/net/ethernet/amd/pcnet32.c
@@ -2818,7 +2818,6 @@ static void pcnet32_remove_one(struct pci_dev *pdev)
 				    lp->init_block, lp->init_dma_addr);
 		free_netdev(dev);
 		pci_disable_device(pdev);
-		pci_set_drvdata(pdev, NULL);
 	}
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 03/52] net: starfire: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-11  7:24 UTC (permalink / raw)
  To: 'David S. Miller'
  Cc: netdev, 'Jingoo Han', 'Ion Badulescu'
In-Reply-To: <004a01ceaebf$43f920f0$cbeb62d0$%han@samsung.com>

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/net/ethernet/adaptec/starfire.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 8b04bfc..171d73c 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -835,7 +835,6 @@ static int starfire_init_one(struct pci_dev *pdev,
 	return 0;
 
 err_out_cleardev:
-	pci_set_drvdata(pdev, NULL);
 	iounmap(base);
 err_out_free_res:
 	pci_release_regions (pdev);
@@ -2012,7 +2011,6 @@ static void starfire_remove_one(struct pci_dev *pdev)
 	iounmap(np->base);
 	pci_release_regions(pdev);
 
-	pci_set_drvdata(pdev, NULL);
 	free_netdev(dev);			/* Will also free np!! */
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 02/52] net: 8390: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-11  7:23 UTC (permalink / raw)
  To: 'David S. Miller'
  Cc: netdev, 'Jingoo Han', 'Paul Gortmaker'
In-Reply-To: <004a01ceaebf$43f920f0$cbeb62d0$%han@samsung.com>

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/net/ethernet/8390/ne2k-pci.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/ethernet/8390/ne2k-pci.c b/drivers/net/ethernet/8390/ne2k-pci.c
index 9220108..fc14a85 100644
--- a/drivers/net/ethernet/8390/ne2k-pci.c
+++ b/drivers/net/ethernet/8390/ne2k-pci.c
@@ -389,9 +389,7 @@ err_out_free_netdev:
 	free_netdev (dev);
 err_out_free_res:
 	release_region (ioaddr, NE_IO_EXTENT);
-	pci_set_drvdata (pdev, NULL);
 	return -ENODEV;
-
 }
 
 /*
@@ -655,7 +653,6 @@ static void ne2k_pci_remove_one(struct pci_dev *pdev)
 	release_region(dev->base_addr, NE_IO_EXTENT);
 	free_netdev(dev);
 	pci_disable_device(pdev);
-	pci_set_drvdata(pdev, NULL);
 }
 
 #ifdef CONFIG_PM
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 01/52] net: typhoon: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-11  7:21 UTC (permalink / raw)
  To: 'David S. Miller'
  Cc: netdev, 'Jingoo Han', 'David Dillow'
In-Reply-To: <004a01ceaebf$43f920f0$cbeb62d0$%han@samsung.com>

The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/net/ethernet/3com/typhoon.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/3com/typhoon.c b/drivers/net/ethernet/3com/typhoon.c
index 144942f6..465cc71 100644
--- a/drivers/net/ethernet/3com/typhoon.c
+++ b/drivers/net/ethernet/3com/typhoon.c
@@ -2525,7 +2525,6 @@ typhoon_remove_one(struct pci_dev *pdev)
 	pci_release_regions(pdev);
 	pci_clear_mwi(pdev);
 	pci_disable_device(pdev);
-	pci_set_drvdata(pdev, NULL);
 	free_netdev(dev);
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 00/52] net: ethernet: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-09-11  7:19 UTC (permalink / raw)
  To: 'David S. Miller'; +Cc: netdev, 'Jingoo Han'

Since commit 0998d0631001288a5974afc0b2a5f568bcdecb4d
(device-core: Ensure drvdata = NULL when no driver is bound),
the driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.

---
 drivers/net/ethernet/3com/typhoon.c                  |    1 -
 drivers/net/ethernet/8390/ne2k-pci.c                 |    3 ---
 drivers/net/ethernet/adaptec/starfire.c              |    2 --
 drivers/net/ethernet/amd/amd8111e.c                  |    2 --
 drivers/net/ethernet/amd/pcnet32.c                   |    1 -
 drivers/net/ethernet/atheros/alx/main.c              |    1 -
 drivers/net/ethernet/broadcom/bnx2.c                 |    3 ---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c     |    3 ---
 drivers/net/ethernet/broadcom/tg3.c                  |    2 --
 drivers/net/ethernet/brocade/bna/bnad.c              |    1 -
 drivers/net/ethernet/chelsio/cxgb/cxgb2.c            |    2 --
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c      |    2 --
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c      |    5 ++---
 drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c  |    3 ---
 drivers/net/ethernet/cisco/enic/enic_main.c          |    2 --
 drivers/net/ethernet/dec/tulip/de2104x.c             |    1 -
 drivers/net/ethernet/dec/tulip/dmfe.c                |    3 ---
 drivers/net/ethernet/dec/tulip/tulip_core.c          |    1 -
 drivers/net/ethernet/dec/tulip/uli526x.c             |    2 --
 drivers/net/ethernet/dec/tulip/winbond-840.c         |    3 ---
 drivers/net/ethernet/dec/tulip/xircom_cb.c           |    2 --
 drivers/net/ethernet/dlink/dl2k.c                    |    1 -
 drivers/net/ethernet/dlink/sundance.c                |    2 --
 drivers/net/ethernet/emulex/benet/be_main.c          |    2 --
 drivers/net/ethernet/fealnx.c                        |    4 ++--
 drivers/net/ethernet/icplus/ipg.c                    |    1 -
 drivers/net/ethernet/intel/e100.c                    |    2 --
 drivers/net/ethernet/jme.c                           |    2 --
 drivers/net/ethernet/marvell/skge.c                  |    2 --
 drivers/net/ethernet/marvell/sky2.c                  |    3 ---
 drivers/net/ethernet/micrel/ksz884x.c                |    2 --
 drivers/net/ethernet/myricom/myri10ge/myri10ge.c     |    1 -
 drivers/net/ethernet/natsemi/natsemi.c               |    2 --
 drivers/net/ethernet/neterion/s2io.c                 |    2 --
 drivers/net/ethernet/neterion/vxge/vxge-main.c       |    2 --
 drivers/net/ethernet/packetengines/hamachi.c         |    1 -
 drivers/net/ethernet/packetengines/yellowfin.c       |    2 --
 drivers/net/ethernet/pasemi/pasemi_mac.c             |    1 -
 drivers/net/ethernet/qlogic/netxen/netxen_nic_main.c |    2 --
 drivers/net/ethernet/qlogic/qla3xxx.c                |    2 --
 drivers/net/ethernet/qlogic/qlcnic/qlcnic_main.c     |    2 --
 drivers/net/ethernet/qlogic/qlge/qlge_main.c         |    1 -
 drivers/net/ethernet/rdc/r6040.c                     |    2 --
 drivers/net/ethernet/realtek/8139cp.c                |    1 -
 drivers/net/ethernet/realtek/8139too.c               |    1 -
 drivers/net/ethernet/realtek/r8169.c                 |    1 -
 drivers/net/ethernet/sis/sis190.c                    |    1 -
 drivers/net/ethernet/smsc/epic100.c                  |    1 -
 drivers/net/ethernet/smsc/smsc9420.c                 |    2 --
 drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c     |    1 -
 drivers/net/ethernet/sun/cassini.c                   |    2 --
 drivers/net/ethernet/sun/niu.c                       |    2 --
 drivers/net/ethernet/sun/sungem.c                    |    2 --
 drivers/net/ethernet/sun/sunhme.c                    |    2 --
 drivers/net/ethernet/tehuti/tehuti.c                 |    1 -
 drivers/net/ethernet/ti/tlan.c                       |    1 -
 drivers/net/ethernet/toshiba/spider_net.c            |    1 -
 drivers/net/ethernet/toshiba/tc35815.c               |    1 -
 drivers/net/ethernet/via/via-rhine.c                 |    1 -
 59 files changed, 4 insertions(+), 103 deletions(-)

^ permalink raw reply

* Re: [PATCH] xen-netback: fix possible format string flaw
From: Ian Campbell @ 2013-09-11  7:18 UTC (permalink / raw)
  To: Kees Cook; +Cc: linux-kernel, xen-devel, netdev
In-Reply-To: <20130911043911.GA17237@www.outflux.net>

On Tue, 2013-09-10 at 21:39 -0700, Kees Cook wrote:
> This makes sure a format string cannot accidentally leak into the
> kthread_run() call.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Acked-by: Ian Campbell <ian.campbell@citrix.com>

Thanks.

Ian.

^ permalink raw reply

* Re: [PATCH] ipv6: Do route updating for redirect in ndisc layer
From: Duan Jiong @ 2013-09-11  7:04 UTC (permalink / raw)
  To: hannes; +Cc: davem, netdev
In-Reply-To: <20130910225023.GB4794@order.stressinduktion.org>

于 2013年09月11日 06:50, Hannes Frederic Sowa 写道:
> On Mon, Sep 09, 2013 at 03:09:56PM +0800, Duan Jiong wrote:
>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>> index 5c71501..61fe8e5 100644
>> --- a/net/ipv6/tcp_ipv6.c
>> +++ b/net/ipv6/tcp_ipv6.c
>> @@ -382,14 +382,6 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>>  
>>  	np = inet6_sk(sk);
>>  
>> -	if (type == NDISC_REDIRECT) {
>> -		struct dst_entry *dst = __sk_dst_check(sk, np->dst_cookie);
>> -
>> -		if (dst)
>> -			dst->ops->redirect(dst, sk, skb);
>> -		goto out;
>> -	}
>> -
> 
> You dropped the "goto out" here in case of an NDISC_REDIRECT, so this sends an
> EPROTO further up the socket layer. Was this intended?
> 

I'm sorry, i didn't notice the variable err was assigned to EPROTO.
I only thought that message should be sent to the socket layer, because
i found that in function sctp_v6_err().

In addition, the rfc 4443 said the Redirect Message is not the ICMPv6 Error
Message, so i think we shouldn't call those err_handler function, in other
words we shouldn't call the icmpv6_notify().

How do you think of this?

> Also:
> 
> In some _err() functions there is this check, e.g. ah6.c:
> 
>     621         if (type != ICMPV6_DEST_UNREACH &&
>     622             type != ICMPV6_PKT_TOOBIG &&
>     623             type != NDISC_REDIRECT)
>     624                 return;
> 
> It could actually be adjusted now as we don't handle NDISC_REDIRECTs here any
> more. I don't see any side-effects down the code in these functions. We could
> also only just match on ICMPV6_PKT_TOOBIG. Can you confirm?
> 

Yes, i agree this. 

Thanks,
  Duan

^ permalink raw reply

* Re: [PATCH net 2/4] bridge: Handle priority-tagged frames properly
From: Toshiaki Makita @ 2013-09-11  7:00 UTC (permalink / raw)
  To: vyasevic; +Cc: David S. Miller, netdev
In-Reply-To: <522F26B3.60709@redhat.com>

On Tue, 2013-09-10 at 10:03 -0400, Vlad Yasevich wrote:
> On 09/10/2013 06:34 AM, Toshiaki Makita wrote:
> > IEEE 802.1Q says that when we receive priority-tagged (VID 0) frames
> > use the PVID for the port as its VID.
> > (See IEEE 802.1Q-2005 6.7.1 and Table 9-2)
> >
> > Apply the PVID to not only untagged frames but also priority-tagged frames.
> >
> > Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> > ---
> >   net/bridge/br_vlan.c | 27 ++++++++++++++++++++-------
> >   1 file changed, 20 insertions(+), 7 deletions(-)
> >
> > diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> > index 21b6d21..5a9c44a 100644
> > --- a/net/bridge/br_vlan.c
> > +++ b/net/bridge/br_vlan.c
> > @@ -189,6 +189,8 @@ out:
> >   bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
> >   			struct sk_buff *skb, u16 *vid)
> >   {
> > +	int err;
> > +
> >   	/* If VLAN filtering is disabled on the bridge, all packets are
> >   	 * permitted.
> >   	 */
> > @@ -201,20 +203,31 @@ bool br_allowed_ingress(struct net_bridge *br, struct net_port_vlans *v,
> >   	if (!v)
> >   		return false;
> >
> > -	if (br_vlan_get_tag(skb, vid)) {
> > +	err = br_vlan_get_tag(skb, vid);
> > +	if (!*vid) {
> >   		u16 pvid = br_get_pvid(v);
> >
> > -		/* Frame did not have a tag.  See if pvid is set
> > -		 * on this port.  That tells us which vlan untagged
> > -		 * traffic belongs to.
> > +		/* Frame had a tag with VID 0 or did not have a tag.
> > +		 * See if pvid is set on this port.  That tells us which
> > +		 * vlan untagged or priority-tagged traffic belongs to.
> >   		 */
> >   		if (pvid == VLAN_N_VID)
> >   			return false;
> >
> > -		/* PVID is set on this port.  Any untagged ingress
> > -		 * frame is considered to belong to this vlan.
> > +		/* PVID is set on this port.  Any untagged or priority-tagged
> > +		 * ingress frame is considered to belong to this vlan.
> >   		 */
> > -		__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
> > +		if (likely(err))
> > +			/* Untagged Frame. */
> > +			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), pvid);
> > +		else
> > +			/* Priority-tagged Frame.
> > +			 * At this point, We know that skb->vlan_tci had
> > +			 * VLAN_TAG_PRESENT bit and its VID field was 0x000.
> > +			 * We update only VID field and preserve PCP field.
> > +			 */
> > +			skb->vlan_tci |= pvid;
> > +
> 
> In the case of a priority tagged frame, we should unroll the 
> modification above and restore the VID field to 0.  Otherwise, you
> may end up either stripping the vlan header completely or forwarding
> with pvid of the ingress port.

Thank you for reviewing.

It is my intended behavior that an incoming priority-tagged frame is
forwarded as a frame untagged or tagged with pvid.

IEEE 802.1Q-2011:

  section 8.1.7 Conversion of frame formats

  NOTE - As all incoming frames, including priority-tagged frames, are
  classified as belonging to a VLAN, the transmitting Port transmits
  VLAN-tagged frames or untagged frames. Hence, a station sending a
  priority-tagged frame via a Bridge will receive a response that is
  either VLAN-tagged or untagged, as described in 8.5.

  3. Definitions

  3.132 Priority-tagged frame: A tagged frame whose tag header carries
  priority information but carries no VLAN identification information.

  3.203 VLAN-tagged frame: A VLAN-tagged frame is a tagged frame whose
  tag header carries *both* VLAN identification and priority
  information.

Toshiaki Makita

> 
> -vlad
> >   		return true;
> >   	}
> >
> >

^ permalink raw reply

* Re: [RFC PATCHv2 0/4] Add DT support for fixed PHYs
From: Sascha Hauer @ 2013-09-11  6:42 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: David S. Miller, netdev, devicetree, Florian Fainelli,
	Lior Amsalem, Gregory Clement, Ezequiel Garcia, linux-arm-kernel,
	Mark Rutland, Christian Gmeiner
In-Reply-To: <1378480701-12908-1-git-send-email-thomas.petazzoni@free-electrons.com>

On Fri, Sep 06, 2013 at 05:18:17PM +0200, Thomas Petazzoni wrote:
> Hello,
> 
> Here is a second version of the patch set that adds a Device Tree
> binding and the related code to support fixed PHYs. Marked as RFC,
> this patch set is obviously not intended for merging in 3.12.
> 
> Since the first version, the changes have been:
> 
>  * Instead of using a 'fixed-link' property inside the Ethernet device
>    DT node, with a fairly cryptic succession of integer values, we now
>    use a PHY subnode under the Ethernet device DT node, with explicit
>    properties to configure the duplex, speed, pause and other PHY
>    properties.
> 
>  * The PHY address is automatically allocated by the kernel and no
>    longer visible in the Device Tree binding.
> 
>  * The PHY device is created directly when the network driver calls
>    of_phy_connect_fixed_link(), and associated to the PHY DT node,
>    which allows the existing of_phy_connect() function to work,
>    without the need to use the deprecated of_phy_connect_fixed_link().
> 
> The things I am not entirely happy with yet are:
> 
>  * The PHY ID is hardcoded to 0xdeadbeef. Ideally, it should be a
>    properly reserved vendor/device identifier, but it isn't clear how
>    to get one allocated for this purpose.
> 
>  * The fixed_phy_register() function in drivers/net/phy/fixed.c has
>    some OF references. So ideally, I would have preferred to put this
>    code in drivers/of/of_mdio.c, but to call get_phy_device(), we need
>    a reference to the mii_bus structure that represents the fixed MDIO
>    bus.
> 
>  * There is some error management missing in fixed_phy_register(), but
>    it can certainly be added easily. This RFC is meant to sort out the
>    general idea.

+1 for the general idea. This really looks good now. I've not much more
to say. Maybe someone from the devicetree corner has a few words for the
binding?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH] net_sched: htb: fix a typo in htb_change_class()
From: Jiri Pirko @ 2013-09-11  5:58 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Jesper Dangaard Brouer, Vimalkumar,
	Jiri Pirko
In-Reply-To: <1378859797.26319.90.camel@edumazet-glaptop>

Wed, Sep 11, 2013 at 02:36:37AM CEST, eric.dumazet@gmail.com wrote:
>From: Vimalkumar <j.vimal@gmail.com>
>
>Fix a typo added in commit 56b765b79 ("htb: improved accuracy at high
>rates")
>
>cbuffer should not be a copy of buffer.
>
>Signed-off-by: Vimalkumar <j.vimal@gmail.com>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Jesper Dangaard Brouer <brouer@redhat.com>
>Cc: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Jiri Pirko <jiri@resnulli.us>

^ permalink raw reply

* Re: [PATCH v2 net-next 08/27] bonding: remove bond_for_each_slave_reverse()
From: Veaceslav Falico @ 2013-09-11  5:39 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <l0oiea$jo6$2@ger.gmane.org>

On Wed, Sep 11, 2013 at 3:53 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, 10 Sep 2013 at 20:57 GMT, Veaceslav Falico <vfalico@redhat.com> wrote:
>> We only use it in rollback scenarios and can easily use the standart
>> bond_for_each_dev() instead.
>>
>
> What you remove actually is bond_for_each_slave_continue_reverse()...
> $subject needs to be fixed.

Indeed, thank you, will fix :).

>
>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index c3dcc6b..46f6b40 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -1246,9 +1246,9 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
>>   */
>>  static int alb_set_mac_address(struct bonding *bond, void *addr)
>>  {
>> -     char tmp_addr[ETH_ALEN];
>> -     struct slave *slave;
>> +     struct slave *slave, *rollback_slave;
>>       struct sockaddr sa;
>> +     char tmp_addr[ETH_ALEN];
>
> Why are you moving tmp_addr[]?

It's more readable, IMO, if structs go first.

-- 
Best regards,
Veaceslav Falico

^ permalink raw reply

* Re: [PATCH v2 net-next 07/27] net: add for_each iterators through neighbour lower link's private
From: Veaceslav Falico @ 2013-09-11  5:37 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev
In-Reply-To: <l0oi1g$jo6$1@ger.gmane.org>

On Wed, Sep 11, 2013 at 3:46 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, 10 Sep 2013 at 20:57 GMT, Veaceslav Falico <vfalico@redhat.com> wrote:
>> +void *netdev_lower_get_next_private_rcu(struct net_device *dev,
>> +                                     struct list_head **iter)
>> +{
>> +     struct netdev_adjacent *lower;
>> +
>> +     WARN_ON_ONCE(!rcu_read_lock_held());
>> +
>
> Since this function has _rcu suffix, this warning is not useful at all.

I've been using it to catch offenders, just in case, like BUG_ON.

I can easily remove it, if needed, there are *tons* of cleanups
that can be made for this patchset, tbh, I've just left it the way it
is to be able to catch bug/review easier :).

p.s. Please, use reply-all when replying...

-- 
Best regards,
Veaceslav Falico

^ permalink raw reply

* Re: Use-after-free in TUNSETIFF
From: Jason Wang @ 2013-09-11  4:45 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Wannes Rombouts, davem, edumazet, nhorman, netdev, Kevin Soules
In-Reply-To: <20130911044355.GB13891@redhat.com>



----- Original Message -----
> On Wed, Sep 11, 2013 at 01:59:47AM +0200, Wannes Rombouts wrote:
> > (I sent this email to security@kernel.org but they told me I should send
> > it to you guys, so here you go.)
> > 
> > Hi,
> > 
> > I would like to report what I believe could be a potential CAP_NET_ADMIN
> > to ring0 privilege escalation.
> > 
> > The bug is in the way tuntap interfaces are initialized, when given an
> > invalid name they cause a use after free. Also software like vmware
> > allows for at least a freeze or kernel panic by a simple user but might
> > also allow privilege escalation.
> > 
> > Very simple to test, this causes a crash:
> > # ip tuntap add dev %% mode tap
> > If it doesn't crash immediately wait a few seconds and try again.
> > 
> > 
> > We haven't managed to exploit the use after free yet, but we are still
> > working on it. At least it crashes even with the latest kernel 3.11 and
> > on different distros. (tested on Debian, Ubuntu and Arch) Looking at the
> > source the bug seems quite old.
> > 
> > 
> > Here is our analysis:
> > 
> > A user with CAP_NET_ADMIN calls ioctl with TUNSETIFF and an invalid name
> > for example "%d%d".
> > 
> > tun_set_iff starts to initialize the tun_struct.
> > http://lxr.free-electrons.com/source/drivers/net/tun.c#L1589
> > 
> > It calls tun_flow_init which starts a timer with tun_flow_cleanup as
> > callback. http://lxr.free-electrons.com/source/drivers/net/tun.c#L852
> > 
> > After this tun_set_iff calls register_netdevice which returns an error
> > because of the invalid name.
> > 
> > This error causes the goto err_free_dev and the call to free_netdev.
> > This will free the tun_struct.
> > 
> > Later, once the callback gets called it uses bad memory. Sometimes it
> > doesn’t get called because the timer_list has been compromised and we
> > get a kernel panic at:
> > http://lxr.free-electrons.com/source/kernel/timer.c?v=2.6.33#L949
> > 
> > But it is possible to get some memory from userland that overlaps only
> > the beginning of the tun_struct without overwriting the timer_list
> > because there is a big array before it. Then it might be possible to
> > exploit tun_flow_cleanup when it is called, but we didn't succeed yet.
> > 
> > ------------------------------------------------------------------------
> > 
> > 
> > This is the first time we try to exploit the kernel so we basically suck
> > at this. I don't know if someone more skilled could do this easily or
> > not, but we'll keep trying and I'll let you know if we manage it.
> > 
> > In the mean time please let us know what you think of this and of course
> > we are very interested in the way this is patched. Please keep us in the
> > loop.
> > 
> > Of course we will be happy to assist in any way we can, feel free to
> > ask! Also we would like to know when you think it would be reasonable to
> > disclose and talk about this bug.
> > 
> > Regards,
> > 
> > Wannes 'wapiflapi' Rombouts
> > Kevin 'eax64' Soules
> > 
> > 
> >
> 
> Thanks a lot for the report. So this one is easy to fix I think.
> Does the below patch help?
> However, looking at the error handling in that function,
> it looks like it could leak resources in many other ways.
> We probably need more patches on top to fix it properly.
> 

True, I'm working on a patch to solve all of them. Will post soon

^ permalink raw reply

* Re: [PATCH 1/1] sfc: Staticize efx_ethtool_get_ts_info
From: Sachin Kamat @ 2013-09-11  4:43 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: netdev@vger.kernel.org, linux-net-drivers
In-Reply-To: <1378814891.32513.29.camel@deadeye.wl.decadent.org.uk>

On 10 September 2013 17:38, Ben Hutchings <bhutchings@solarflare.com> wrote:
> On Tue, 2013-09-10 at 15:13 +0530, Sachin Kamat wrote:
>> 'efx_ethtool_get_ts_info' is used only in this file.
>> Make it static.

>
> I already have this fix, but net-next is not open for changes.

I wasn't aware. Sorry for the noise.

-- 
With warm regards,
Sachin

^ permalink raw reply

* Re: Use-after-free in TUNSETIFF
From: Michael S. Tsirkin @ 2013-09-11  4:43 UTC (permalink / raw)
  To: Wannes Rombouts; +Cc: davem, jasowang, edumazet, nhorman, netdev, Kevin Soules
In-Reply-To: <522FB273.1030506@epitech.eu>

On Wed, Sep 11, 2013 at 01:59:47AM +0200, Wannes Rombouts wrote:
> (I sent this email to security@kernel.org but they told me I should send
> it to you guys, so here you go.)
> 
> Hi,
> 
> I would like to report what I believe could be a potential CAP_NET_ADMIN
> to ring0 privilege escalation.
> 
> The bug is in the way tuntap interfaces are initialized, when given an
> invalid name they cause a use after free. Also software like vmware
> allows for at least a freeze or kernel panic by a simple user but might
> also allow privilege escalation.
> 
> Very simple to test, this causes a crash:
> # ip tuntap add dev %% mode tap
> If it doesn't crash immediately wait a few seconds and try again.
> 
> 
> We haven't managed to exploit the use after free yet, but we are still
> working on it. At least it crashes even with the latest kernel 3.11 and
> on different distros. (tested on Debian, Ubuntu and Arch) Looking at the
> source the bug seems quite old.
> 
> 
> Here is our analysis:
> 
> A user with CAP_NET_ADMIN calls ioctl with TUNSETIFF and an invalid name
> for example "%d%d".
> 
> tun_set_iff starts to initialize the tun_struct.
> http://lxr.free-electrons.com/source/drivers/net/tun.c#L1589
> 
> It calls tun_flow_init which starts a timer with tun_flow_cleanup as
> callback. http://lxr.free-electrons.com/source/drivers/net/tun.c#L852
> 
> After this tun_set_iff calls register_netdevice which returns an error
> because of the invalid name.
> 
> This error causes the goto err_free_dev and the call to free_netdev.
> This will free the tun_struct.
> 
> Later, once the callback gets called it uses bad memory. Sometimes it
> doesn’t get called because the timer_list has been compromised and we
> get a kernel panic at:
> http://lxr.free-electrons.com/source/kernel/timer.c?v=2.6.33#L949
> 
> But it is possible to get some memory from userland that overlaps only
> the beginning of the tun_struct without overwriting the timer_list
> because there is a big array before it. Then it might be possible to
> exploit tun_flow_cleanup when it is called, but we didn't succeed yet.
> 
> ------------------------------------------------------------------------
> 
> 
> This is the first time we try to exploit the kernel so we basically suck
> at this. I don't know if someone more skilled could do this easily or
> not, but we'll keep trying and I'll let you know if we manage it.
> 
> In the mean time please let us know what you think of this and of course
> we are very interested in the way this is patched. Please keep us in the
> loop.
> 
> Of course we will be happy to assist in any way we can, feel free to
> ask! Also we would like to know when you think it would be reasonable to
> disclose and talk about this bug.
> 
> Regards,
> 
> Wannes 'wapiflapi' Rombouts
> Kevin 'eax64' Soules
> 
> 
>

Thanks a lot for the report. So this one is easy to fix I think.
Does the below patch help?
However, looking at the error handling in that function,
it looks like it could leak resources in many other ways.
We probably need more patches on top to fix it properly.

-->

tun: cleanup flow on set_iff error path

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index b7c457a..c0470c1 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1660,11 +1660,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		INIT_LIST_HEAD(&tun->disabled);
 		err = tun_attach(tun, file);
 		if (err < 0)
-			goto err_free_dev;
+			goto err_free_flow;
 
 		err = register_netdevice(tun->dev);
 		if (err < 0)
-			goto err_free_dev;
+			goto err_free_flow;
 
 		if (device_create_file(&tun->dev->dev, &dev_attr_tun_flags) ||
 		    device_create_file(&tun->dev->dev, &dev_attr_owner) ||
@@ -1708,7 +1708,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	strcpy(ifr->ifr_name, tun->dev->name);
 	return 0;
 
- err_free_dev:
+err_free_flow:
+	tun_flow_uninit(dev);
+err_free_dev:
 	free_netdev(dev);
 	return err;
 }
 

^ permalink raw reply related

* [PATCH] xen-netback: fix possible format string flaw
From: Kees Cook @ 2013-09-11  4:39 UTC (permalink / raw)
  To: linux-kernel; +Cc: Ian Campbell, xen-devel, netdev

This makes sure a format string cannot accidentally leak into the
kthread_run() call.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/net/xen-netback/interface.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/xen-netback/interface.c b/drivers/net/xen-netback/interface.c
index 625c6f4..77fee1d 100644
--- a/drivers/net/xen-netback/interface.c
+++ b/drivers/net/xen-netback/interface.c
@@ -406,7 +406,7 @@ int xenvif_connect(struct xenvif *vif, unsigned long tx_ring_ref,
 
 	init_waitqueue_head(&vif->wq);
 	vif->task = kthread_create(xenvif_kthread,
-				   (void *)vif, vif->dev->name);
+				   (void *)vif, "%s", vif->dev->name);
 	if (IS_ERR(vif->task)) {
 		pr_warn("Could not allocate kthread for %s\n", vif->dev->name);
 		err = PTR_ERR(vif->task);
-- 
1.7.9.5


-- 
Kees Cook
Chrome OS Security

^ permalink raw reply related

* [PATCH] bug[#43]: kgdboe: netpoll: Should handle ETH_P_ARP other than ETH_P_IP in netpoll_neigh_reply
From: Sonic Zhang @ 2013-09-11  3:31 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, adi-buildroot-devel, Sonic Zhang

From: Sonic Zhang <sonic.zhang@analog.com>

The received ARP request type in the Ethernet packet head is ETH_P_ARP other than ETH_P_IP.

Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
---
 net/core/netpoll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 2c637e9..c3c7b27 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -550,7 +550,7 @@ static void netpoll_neigh_reply(struct sk_buff *skb, struct netpoll_info *npinfo
 		return;
 
 	proto = ntohs(eth_hdr(skb)->h_proto);
-	if (proto == ETH_P_IP) {
+	if (proto == ETH_P_ARP) {
 		struct arphdr *arp;
 		unsigned char *arp_ptr;
 		/* No arp on this interface */
-- 
1.8.2.3

^ permalink raw reply related

* [PATCH v2 1/1] net: fec: fix phy reset operation to let imx6sl evk work
From: Fugang Duan @ 2013-09-11  2:18 UTC (permalink / raw)
  To: shawn.guo, davem; +Cc: netdev, bhutchings, stephen, b20596, s.hauer, b38611

The patch do correct phy reset operation to let imx6sl evk
ethernet work.

Current driver only do phy reset in probe function, which is
not right. Since some phy clock is disabled after module probe,
the phy enter abnormal status, which needs do reset to recovery
the phy. And do ifconfig ethx up/down test, the phy also enter
abnormal status.

The log as:
libphy: 2188000.ethernet:04 - Link is Up - 10/Full
libphy: 2188000.ethernet:04 - Link is Up - 100/Full
libphy: 2188000.ethernet:04 - Link is Down
libphy: 2188000.ethernet:04 - Link is Up - 10/Half
libphy: 2188000.ethernet:04 - Link is Up - 10/Full
libphy: 2188000.ethernet:04 - Link is Up - 100/Full
...

So, do phy reset if ethx up/down or clock enable/disable.

Changes from V1:
 - remove some unecessary comments.
 - move the function "fec_enet_reset_phy()" up and remove
   the prototype for the function.

Signed-off-by: Fugang Duan <B38611@freescale.com>
Reviewed-by: Shawn Guo <R65073@freescale.com>
CC: Shawn Guo <shawn.guo@linaro.org>
CC: Li Frank <B20596@freescale.com>
CC: "David S. Miller" <davem@davemloft.net>
CC: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/ethernet/freescale/fec.h      |    3 +
 drivers/net/ethernet/freescale/fec_main.c |   60 +++++++++++++++++------------
 2 files changed, 38 insertions(+), 25 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 0120217..473d5fb 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -321,6 +321,9 @@ struct fec_enet_private {
 	struct	napi_struct napi;
 	int	csum_flags;
 
+	int	phy_reset_gpio;
+	int	reset_duration;
+
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
 	unsigned long last_overflow_check;
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index f9aacf5..153dcbf 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -238,6 +238,18 @@ MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
 
 static int mii_cnt;
 
+static void fec_enet_reset_phy(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct fec_enet_private *fep = netdev_priv(ndev);
+
+	if (gpio_is_valid(fep->phy_reset_gpio)) {
+		gpio_set_value(fep->phy_reset_gpio, 0);
+		msleep(fep->reset_duration);
+		gpio_set_value(fep->phy_reset_gpio, 1);
+	}
+}
+
 static inline
 struct bufdesc *fec_enet_get_nextdesc(struct bufdesc *bdp, struct fec_enet_private *fep)
 {
@@ -1780,6 +1792,9 @@ fec_enet_open(struct net_device *ndev)
 	phy_start(fep->phy_dev);
 	netif_start_queue(ndev);
 	fep->opened = 1;
+
+	fec_enet_reset_phy(fep->pdev);
+
 	return 0;
 }
 
@@ -2029,43 +2044,39 @@ static int fec_enet_init(struct net_device *ndev)
 	return 0;
 }
 
-#ifdef CONFIG_OF
-static void fec_reset_phy(struct platform_device *pdev)
+static void fec_enet_of_init(struct platform_device *pdev)
 {
-	int err, phy_reset;
-	int msec = 1;
 	struct device_node *np = pdev->dev.of_node;
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct fec_enet_private *fep = netdev_priv(ndev);
+	int err;
+
+	/*
+	 * init phy-reset-gpio to one invalid GPIO for no phy
+	 * gpio reset platform
+	 */
+	fep->phy_reset_gpio = -1;
 
 	if (!np)
 		return;
 
-	of_property_read_u32(np, "phy-reset-duration", &msec);
+	of_property_read_u32(np, "phy-reset-duration",
+				&fep->reset_duration);
 	/* A sane reset duration should not be longer than 1s */
-	if (msec > 1000)
-		msec = 1;
+	if ((fep->reset_duration > 1000) || (fep->reset_duration == 0))
+		fep->reset_duration = 1;
 
-	phy_reset = of_get_named_gpio(np, "phy-reset-gpios", 0);
-	if (!gpio_is_valid(phy_reset))
+	fep->phy_reset_gpio = of_get_named_gpio(np, "phy-reset-gpios", 0);
+	if (!gpio_is_valid(fep->phy_reset_gpio))
 		return;
 
-	err = devm_gpio_request_one(&pdev->dev, phy_reset,
-				    GPIOF_OUT_INIT_LOW, "phy-reset");
+	err = devm_gpio_request_one(&pdev->dev, fep->phy_reset_gpio,
+				GPIOF_OUT_INIT_HIGH, "phy-reset");
 	if (err) {
 		dev_err(&pdev->dev, "failed to get phy-reset-gpios: %d\n", err);
-		return;
+		fep->phy_reset_gpio = -1;
 	}
-	msleep(msec);
-	gpio_set_value(phy_reset, 1);
 }
-#else /* CONFIG_OF */
-static void fec_reset_phy(struct platform_device *pdev)
-{
-	/*
-	 * In case of platform probe, the reset has been done
-	 * by machine code.
-	 */
-}
-#endif /* CONFIG_OF */
 
 static int
 fec_probe(struct platform_device *pdev)
@@ -2113,6 +2124,7 @@ fec_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, ndev);
 
+	fec_enet_of_init(pdev);
 	ret = of_get_phy_mode(pdev->dev.of_node);
 	if (ret < 0) {
 		pdata = dev_get_platdata(&pdev->dev);
@@ -2181,8 +2193,6 @@ fec_probe(struct platform_device *pdev)
 		fep->reg_phy = NULL;
 	}
 
-	fec_reset_phy(pdev);
-
 	if (fep->bufdesc_ex)
 		fec_ptp_init(pdev);
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH] alx: remove redundant D0 power state set
From: Yijing Wang @ 2013-09-11  2:07 UTC (permalink / raw)
  To: Jay Cliburn, Chris Snook, David S. Miller; +Cc: netdev, Yijing Wang, Hanjun Guo

Pci_enable_device_mem() will set device power state to D0,
so it's no need to do it again in alx_probe().
Also remove redundant PM Cap find code, because pci core
has been saved the pci device pm cap value.

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/net/ethernet/atheros/alx/main.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 027398e..fc95b23 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1188,7 +1188,7 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct alx_priv *alx;
 	struct alx_hw *hw;
 	bool phy_configured;
-	int bars, pm_cap, err;
+	int bars, err;
 
 	err = pci_enable_device_mem(pdev);
 	if (err)
@@ -1225,18 +1225,13 @@ static int alx_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	pci_enable_pcie_error_reporting(pdev);
 	pci_set_master(pdev);
 
-	pm_cap = pci_find_capability(pdev, PCI_CAP_ID_PM);
-	if (pm_cap == 0) {
+	if (!pdev->pm_cap) {
 		dev_err(&pdev->dev,
 			"Can't find power management capability, aborting\n");
 		err = -EIO;
 		goto out_pci_release;
 	}
 
-	err = pci_set_power_state(pdev, PCI_D0);
-	if (err)
-		goto out_pci_release;
-
 	netdev = alloc_etherdev(sizeof(*alx));
 	if (!netdev) {
 		err = -ENOMEM;
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v2 net-next 08/27] bonding: remove bond_for_each_slave_reverse()
From: Cong Wang @ 2013-09-11  1:53 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1378846691-9717-9-git-send-email-vfalico@redhat.com>

On Tue, 10 Sep 2013 at 20:57 GMT, Veaceslav Falico <vfalico@redhat.com> wrote:
> We only use it in rollback scenarios and can easily use the standart
> bond_for_each_dev() instead.
>

What you remove actually is bond_for_each_slave_continue_reverse()...
$subject needs to be fixed.


> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
> index c3dcc6b..46f6b40 100644
> --- a/drivers/net/bonding/bond_alb.c
> +++ b/drivers/net/bonding/bond_alb.c
> @@ -1246,9 +1246,9 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
>   */
>  static int alb_set_mac_address(struct bonding *bond, void *addr)
>  {
> -	char tmp_addr[ETH_ALEN];
> -	struct slave *slave;
> +	struct slave *slave, *rollback_slave;
>  	struct sockaddr sa;
> +	char tmp_addr[ETH_ALEN];

Why are you moving tmp_addr[]?

^ permalink raw reply

* Re: [PATCH v2 net-next 07/27] net: add for_each iterators through neighbour lower link's private
From: Cong Wang @ 2013-09-11  1:46 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1378846691-9717-8-git-send-email-vfalico@redhat.com>

On Tue, 10 Sep 2013 at 20:57 GMT, Veaceslav Falico <vfalico@redhat.com> wrote:
> +void *netdev_lower_get_next_private_rcu(struct net_device *dev,
> +					struct list_head **iter)
> +{
> +	struct netdev_adjacent *lower;
> +
> +	WARN_ON_ONCE(!rcu_read_lock_held());
> +

Since this function has _rcu suffix, this warning is not useful at all.

^ permalink raw reply

* RE: [PATCH] net: fec: fix phy reset operation to let imx6sl evk work
From: Duan Fugang-B38611 @ 2013-09-11  1:17 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: shawn.guo@linaro.org, davem@davemloft.net, netdev@vger.kernel.org,
	bhutchings@solarflare.com, stephen@networkplumber.org,
	Li Frank-B20596
In-Reply-To: <20130910184856.GF30088@pengutronix.de>

From: Sascha Hauer [mailto:s.hauer@pengutronix.de]
Data: Wednesday, September 11, 2013 2:49 AM

> To: Duan Fugang-B38611
> Cc: shawn.guo@linaro.org; davem@davemloft.net; netdev@vger.kernel.org;
> bhutchings@solarflare.com; stephen@networkplumber.org; Li Frank-B20596
> Subject: Re: [PATCH] net: fec: fix phy reset operation to let imx6sl evk
> work
> 
> On Tue, Sep 10, 2013 at 05:07:33PM +0800, Fugang Duan wrote:
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index f9aacf5..0c17df2 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -61,6 +61,7 @@
> >  #include "fec.h"
> >
> >  static void set_multicast_list(struct net_device *ndev);
> > +static void fec_enet_reset_phy(struct platform_device *pdev);
> 
> Please move the function up and remove the prototype.
Ok, i will do it. Thanks.

> 
> > @@ -1780,6 +1781,10 @@ fec_enet_open(struct net_device *ndev)
> >  	phy_start(fep->phy_dev);
> >  	netif_start_queue(ndev);
> >  	fep->opened = 1;
> > +
> > +	/* reset phy */
> > +	fec_enet_reset_phy(fep->pdev);
> 
> Drop this comment. It's very redundant to the function name.
Thanks, I will remove the comment.

> 
> > +static void fec_enet_reset_phy(struct platform_device *pdev)
> >  {
> > -	/*
> > -	 * In case of platform probe, the reset has been done
> > -	 * by machine code.
> > -	 */
> > +	struct net_device *ndev = platform_get_drvdata(pdev);
> > +	struct fec_enet_private *fep = netdev_priv(ndev);
> > +
> > +	/* check GPIO valid to avoid kernel print warning when no gpio reset
> */
> > +	if (gpio_is_valid(fep->phy_reset_gpio)) {
> > +		gpio_set_value(fep->phy_reset_gpio, 0);
> > +		msleep(fep->reset_duration);
> > +		gpio_set_value(fep->phy_reset_gpio, 1);
> > +	}
> 
> Drop the comment. It's obvious why it's a good idea to check for a valid
> gpio.

Thanks, I will remove the comments.

^ permalink raw reply

* RE: [PATCH] net: fec: add the initial to set the physical mac address
From: Duan Fugang-B38611 @ 2013-09-11  1:15 UTC (permalink / raw)
  To: Denis Kirjanov
  Cc: Li Frank-B20596, Zhou Luwei-B45643, davem@davemloft.net,
	netdev@vger.kernel.org, shawn.guo@linaro.org,
	bhutchings@solarflare.com, Estevam Fabio-R49496,
	stephen@networkplumber.org
In-Reply-To: <CAOJe8K2eH+6Eg0vOnOFwNsy9-TyxBJwYDCTDbLcbB3yu42OW1w@mail.gmail.com>

From: Denis Kirjanov [mailto:kda@linux-powerpc.org]
Data: Tuesday, September 10, 2013 10:19 PM

> To: Duan Fugang-B38611
> Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net;
> netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com;
> Estevam Fabio-R49496; stephen@networkplumber.org
> Subject: Re: [PATCH] net: fec: add the initial to set the physical mac
> address
> 
> On 8/22/13, Fugang Duan <B38611@freescale.com> wrote:
> > For imx6slx evk platform, the fec driver cannot work since there have
> > no valid mac address set in physical mac registers.
> >
> > For imx serial platform, fec/enet IPs both need the physical MAC
> > address initial, otherwise it cannot work.
> >
> > After acquiring the valid MAC address from devices tree/pfuse/ kernel
> > command line/random mac address, and then set it to the physical MAC
> > address registers.
> >
> > Signed-off-by: Fugang Duan  <B38611@freescale.com>
> > ---
> >  drivers/net/ethernet/freescale/fec_main.c |    9 ++++++---
> >  1 files changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 4349a9e..9b5e08c 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev,
> > void
> > *p)
> >  	struct fec_enet_private *fep = netdev_priv(ndev);
> >  	struct sockaddr *addr = p;
> >
> > -	if (!is_valid_ether_addr(addr->sa_data))
> > -		return -EADDRNOTAVAIL;
> > +	if (p) {
> 
> I don't see how the p pointer can be NULL...

As below, fec_enet_init() function calls fec_set_mac_address(ndev, NULL).

> 
> > +		if (!is_valid_ether_addr(addr->sa_data))
> > +			return -EADDRNOTAVAIL;
> >
> > -	memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > +		memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > +	}
> >
> >  	writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> >  		(ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), @@ -
> 1969,6
> > +1971,7 @@ static int fec_enet_init(struct net_device *ndev)
> >
> >  	/* Get the Ethernet address */
> >  	fec_get_mac(ndev);
> > +	fec_set_mac_address(ndev, NULL);
> >
> >  	/* Set receive and transmit descriptor base. */
> >  	fep->rx_bd_base = cbd_base;

^ permalink raw reply

* Re: [PATCH] bcm63xx_enet: remove deprecated IRQF_DISABLED
From: Jingoo Han @ 2013-09-11  1:05 UTC (permalink / raw)
  To: 'Michael Opdenacker'
  Cc: davem, jogo, joe, mbizon, netdev, linux-kernel
In-Reply-To: <1378537010-7128-1-git-send-email-michael.opdenacker@free-electrons.com>

On Saturday, September 07, 2013 3:57 PM, Michael Opdenacker wrote:
> 
> This patch proposes to remove the IRQF_DISABLED flag from
> drivers/net/ethernet/broadcom/bcm63xx_enet.c
> 
> It's a NOOP since 2.6.35 and it will be removed one day.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/net/ethernet/broadcom/bcm63xx_enet.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

^ permalink raw reply

* Re: [PATCH] net: korina: remove deprecated IRQF_DISABLED
From: Jingoo Han @ 2013-09-11  1:00 UTC (permalink / raw)
  To: 'Michael Opdenacker'
  Cc: davem, emilio, mugunthanvnm, hsweeten, netdev, linux-kernel
In-Reply-To: <1378531257-5446-1-git-send-email-michael.opdenacker@free-electrons.com>

On Saturday, September 07, 2013 2:21 PM, Michael Opdenacker wrote:
> 
> This patch proposes to remove the IRQF_DISABLED flag from
> drivers/net/ethernet/korina.c
> 
> It's a NOOP since 2.6.35 and it will be removed one day.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@free-electrons.com>

Reviewed-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/net/ethernet/korina.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

^ 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