Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 2/2] virtio-net: introduce a new control to set macaddr
From: Jason Wang @ 2013-01-16  6:20 UTC (permalink / raw)
  To: virtualization; +Cc: kvm, mst, netdev, qemu-devel, davem
In-Reply-To: <1358315821-29519-3-git-send-email-akong@redhat.com>

On Wednesday, January 16, 2013 01:57:01 PM akong@redhat.com wrote:
> From: Amos Kong <akong@redhat.com>
> 
> Currently we write MAC address to pci config space byte by byte,
> this means that we have an intermediate step where mac is wrong.
> This patch introduced a new control command to set MAC address
> in one time.
> 
> VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  drivers/net/virtio_net.c        | 24 +++++++++++++++++-------
>  include/uapi/linux/virtio_net.h |  8 +++++++-
>  2 files changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 395ab4f..c8901b6 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -802,16 +802,25 @@ static int virtnet_set_mac_address(struct net_device
> *dev, void *p) struct virtnet_info *vi = netdev_priv(dev);
>  	struct virtio_device *vdev = vi->vdev;
>  	int ret;
> +	struct sockaddr *addr = p;
> +	struct scatterlist sg;
> 
> -	ret = eth_mac_addr(dev, p);
> -	if (ret)
> -		return ret;
> -
> -	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
> +	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> +		sg_init_one(&sg, addr->sa_data, dev->addr_len);
> +		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> +					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
> +					  &sg, 1, 0)) {
> +			dev_warn(&vdev->dev,
> +				 "Failed to set mac address by vq command.\n");
> +			return -EINVAL;
> +		}
> +	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
>  		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
> -		                  dev->dev_addr, dev->addr_len);
> +				  addr->sa_data, dev->addr_len);
> +	}
> +	ret = eth_mac_addr(dev, p);
> 

The you will the validity check in eth_mac_addr which may result a wrong mac 
address to be set in the hardware (or is there any check in qemu) and a 
inconsistency bettween what kernel assumes and qemu has.

You can take a look at netvsc driver that calls eth_mac_addr() first and 
restore the software mac address when fail to enforce it to hardware.

Thanks
> -	return 0;
> +	return ret;
>  }
> 
>  static struct rtnl_link_stats64 *virtnet_stats(struct net_device *dev,
> @@ -1627,6 +1636,7 @@ static unsigned int features[] = {
>  	VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
>  	VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
>  	VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> +	VIRTIO_NET_F_CTRL_MAC_ADDR,
>  };
> 
>  static struct virtio_driver virtio_net_driver = {
> diff --git a/include/uapi/linux/virtio_net.h
> b/include/uapi/linux/virtio_net.h index 848e358..a5a8c88 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -53,6 +53,7 @@
>  					 * network */
>  #define VIRTIO_NET_F_MQ	22	/* Device supports Receive Flow
>  					 * Steering */
> +#define VIRTIO_NET_F_CTRL_MAC_ADDR 23	/* Set MAC address */
> 
>  #define VIRTIO_NET_S_LINK_UP	1	/* Link is up */
>  #define VIRTIO_NET_S_ANNOUNCE	2	/* Announcement is needed */
> @@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
>   #define VIRTIO_NET_CTRL_RX_NOBCAST      5
> 
>  /*
> - * Control the MAC filter table.
> + * Control the MAC
>   *
>   * The MAC filter table is managed by the hypervisor, the guest should
>   * assume the size is infinite.  Filtering should be considered
> @@ -140,6 +141,10 @@ typedef __u8 virtio_net_ctrl_ack;
>   * first sg list contains unicast addresses, the second is for multicast.
>   * This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
>   * is available.
> + *
> + * The ADDR_SET command requests one out scatterlist, it contains a
> + * 6 bytes MAC address. This functionality is present if the
> + * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
>   */
>  struct virtio_net_ctrl_mac {
>  	__u32 entries;
> @@ -148,6 +153,7 @@ struct virtio_net_ctrl_mac {
> 
>  #define VIRTIO_NET_CTRL_MAC    1
>   #define VIRTIO_NET_CTRL_MAC_TABLE_SET        0
> + #define VIRTIO_NET_CTRL_MAC_ADDR_SET         1
> 
>  /*
>   * Control VLAN filtering

^ permalink raw reply

* Re: IPsec AH use of ahash
From: Steffen Klassert @ 2013-01-16  6:21 UTC (permalink / raw)
  To: Tom St Denis; +Cc: herbert, davem, linux-kernel, netdev
In-Reply-To: <528051367.70594.1358268708738.JavaMail.root@elliptictech.com>

Please Cc netdev@vger.kernel.org on all networking related topics.

On Tue, Jan 15, 2013 at 11:51:48AM -0500, Tom St Denis wrote:
> Hi all,
> 
> The AH4/6 code uses ahash to perform the MAC calculation but this precludes the availability of GMAC to the user.  Are there any plans to port the AH code over to the aead API so that it can use potentially any algo available to the user?
> 

There was no need for this by now, but you are welcome
to send patches.

^ permalink raw reply

* Re: per route MTU settings
From: Steffen Klassert @ 2013-01-16  6:31 UTC (permalink / raw)
  To: Lukas Tribus; +Cc: pupilla, netdev
In-Reply-To: <DUB107-W269A7F1946A7B27212F3F8ED2D0@phx.gbl>

On Tue, Jan 15, 2013 at 06:07:59PM +0100, Lukas Tribus wrote:
> 
> Hi pupilla,
> 
> looks like the behavior changed with 3.2-rc5 and "[PATCH 5/5] ipv4:
> Don't use the cached pmtu informations for input routes" ([1], [2]).
> 
> Actually, a "mtu lock XYZ" applied to a route is a bit of a corner case.
> 
> 
> Steffen, you already made this statement once and I can only agree with you:
> 
> > The router that can't send the packet to the next hop network has to
> > send the ICMP Destination Unreachable message. We never propagated
> > learned PMTU informations and I would not like to change this
> 
> 
> But here is our issue:
> - the linux "ip_forwarder" has an MTU of 1500 Byte on relevant interfaces
> - there is a route with a "static" mtu lock at 1200 Byte
> - the box is supposed to forward a packet heading the 1200B MTU route
> 
> What happens is:
> - the packet is dropped (because it exceeds the 1200 Byte)
> - an ICMP Type 3 Code 4 message is generated with 576 Byte next-hop MTU
> 
> Notice that the 576 Byte indicated as next-hop MTU in the ICMP packet
> doesn't match neither outgoing interface MTU, nor the static route's MTU.
> 
> Prior to your patch (for example in 3.2-rc4), 1200 Byte was indicated as
> MTU in the ICMP packet.

This patch was needed during the times we cached the pmtu informations
on the inetpeer. Now the pmtu informations are back in the routes,
so this check is obsolete. We can simply revert it, I'll send a patch
to do that.

^ permalink raw reply

* Re: [RFC(v2) net-next 01/13] ndisc: Refer/Update neigh->updated with write lock.
From: Cong Wang @ 2013-01-16  6:35 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F58777.7080209@linux-ipv6.org>

On 01/16/2013 12:44 AM, YOSHIFUJI Hideaki wrote:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>   net/ipv6/route.c |   12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 7c34c01..1341f68 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -499,22 +499,26 @@ static void rt6_probe(struct rt6_info *rt)
>   	 * to no more than one per minute.
>   	 */
>   	neigh = rt ? rt->n : NULL;
> -	if (!neigh || (neigh->nud_state & NUD_VALID))
> +	if (!neigh)
> +		return;
> +	write_lock_bh(&neigh->lock);
> +	if (neigh->nud_state & NUD_VALID) {
> +		write_unlock_bh(&neigh->lock);
>   		return;
> -	read_lock_bh(&neigh->lock);
> +	}
>   	if (!(neigh->nud_state & NUD_VALID) &&
>   	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
>   		struct in6_addr mcaddr;
>   		struct in6_addr *target;
>   
>   		neigh->updated = jiffies;
> -		read_unlock_bh(&neigh->lock);
> +		write_unlock_bh(&neigh->lock);

This looks like a bug fix, which deserves a separated patch rather than
in this rt->n removal series.

^ permalink raw reply

* [PATCH] ipv4: Remove output route check in ipv4_mtu
From: Steffen Klassert @ 2013-01-16  6:36 UTC (permalink / raw)
  To: David Miller; +Cc: timo.teras, luky-37, pupilla, netdev

The output route check was introduced with git commit 261663b0
(ipv4: Don't use the cached pmtu informations for input routes)
during times when we cached the pmtu informations on the
inetpeer. Now the pmtu informations are back in the routes,
so this check is obsolete. It also had some unwanted side effects,
as reported by Timo Teras and Lukas Tribus.

Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
 net/ipv4/route.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 844a9ef..6e4a89c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1120,7 +1120,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
 	if (!mtu || time_after_eq(jiffies, rt->dst.expires))
 		mtu = dst_metric_raw(dst, RTAX_MTU);
 
-	if (mtu && rt_is_output_route(rt))
+	if (mtu)
 		return mtu;
 
 	mtu = dst->dev->mtu;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [RFC(v2) net-next 07/13] ipv6: Do not repend on rt->n in rt6_probe().
From: Cong Wang @ 2013-01-16  6:42 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F587C2.9060605@linux-ipv6.org>


s/repend/depend/ in $subject...

^ permalink raw reply

* Re: [RFC(v2) net-next 08/13] ipv6: Do not deoend on rt->n in ip6_dst_lookup_tail().
From: Cong Wang @ 2013-01-16  6:49 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F587CF.1090306@linux-ipv6.org>

s/deoend/depend/ in $subject...

On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>   net/ipv6/ip6_output.c |    8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 9fc5d1d..b2fe048 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -921,8 +921,12 @@ static int ip6_dst_lookup_tail(struct sock *sk,
>   	 * dst entry of the nexthop router
>   	 */
>   	rt = (struct rt6_info *) *dst;
> -	n = rt->n;
> -	if (n && !(n->nud_state & NUD_VALID)) {
> +	rcu_read_lock_bh();
> +	n = __ipv6_neigh_lookup_noref(rt->dst.dev, rt6_nexthop(rt, &fl6->daddr));
> +	err = n && !(n->nud_state & NUD_VALID) ? -EINVAL : 0;
> +	rcu_read_unlock_bh();
> +
> +	if (err) {

Are you sure the logic here equals?

In the original code, we could return 0 even we enter this if branch,
but after your patch, it seems it will return -EINVAL?

^ permalink raw reply

* Re: [RFC(v2) net-next 09/13] ipv6: Do not depend on rt->n in rt6_check_neigh().
From: Cong Wang @ 2013-01-16  6:53 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F587D4.6060208@linux-ipv6.org>

On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>   net/ipv6/route.c |   15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index e16a483..ffdc8a6 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -552,20 +552,25 @@ static inline bool rt6_check_neigh(struct rt6_info *rt)
>   	struct neighbour *neigh;
>   	bool ret = false;
>   
> -	neigh = rt->n;
>   	if (rt->rt6i_flags & RTF_NONEXTHOP ||
> -	    !(rt->rt6i_flags & RTF_GATEWAY))
> +	    !(rt->rt6i_flags & RTF_GATEWAY)) {
>   		ret = true;
> -	else if (neigh) {
> -		read_lock_bh(&neigh->lock);
> +		goto out;
> +	}

Just return true here...

^ permalink raw reply

* [PATCH] r8169: remove unneeded dirty_rx index
From: Timo Teräs @ 2013-01-16  7:01 UTC (permalink / raw)
  To: Francois Romieu, netdev; +Cc: Timo Teräs
In-Reply-To: <20130115225316.GA924@electric-eye.fr.zoreil.com>

After commit 6f0333b ("r8169: use 50% less ram for RX ring") the rx
ring buffers are always copied making dirty_rx useless.

Signed-off-by: Timo Teräs <timo.teras@iki.fi>
---
 drivers/net/ethernet/realtek/r8169.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 89184f3..97fdbb1 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -83,7 +83,7 @@ static const int multicast_filter_limit = 32;
 #define R8169_REGS_SIZE		256
 #define R8169_NAPI_WEIGHT	64
 #define NUM_TX_DESC	64	/* Number of Tx descriptor registers */
-#define NUM_RX_DESC	256	/* Number of Rx descriptor registers */
+#define NUM_RX_DESC	256U	/* Number of Rx descriptor registers */
 #define R8169_TX_RING_BYTES	(NUM_TX_DESC * sizeof(struct TxDesc))
 #define R8169_RX_RING_BYTES	(NUM_RX_DESC * sizeof(struct RxDesc))
 
@@ -727,7 +727,6 @@ struct rtl8169_private {
 	u16 mac_version;
 	u32 cur_rx; /* Index into the Rx descriptor buffer of next Rx pkt. */
 	u32 cur_tx; /* Index into the Tx descriptor buffer of next Rx pkt. */
-	u32 dirty_rx;
 	u32 dirty_tx;
 	struct rtl8169_stats rx_stats;
 	struct rtl8169_stats tx_stats;
@@ -4177,7 +4176,7 @@ static void rtl_init_rxcfg(struct rtl8169_private *tp)
 
 static void rtl8169_init_ring_indexes(struct rtl8169_private *tp)
 {
-	tp->dirty_tx = tp->dirty_rx = tp->cur_tx = tp->cur_rx = 0;
+	tp->dirty_tx = tp->cur_tx = tp->cur_rx = 0;
 }
 
 static void rtl_hw_jumbo_enable(struct rtl8169_private *tp)
@@ -5920,7 +5919,7 @@ static void rtl8169_pcierr_interrupt(struct net_device *dev)
 		PCI_STATUS_REC_TARGET_ABORT | PCI_STATUS_SIG_TARGET_ABORT));
 
 	/* The infamous DAC f*ckup only happens at boot time */
-	if ((tp->cp_cmd & PCIDAC) && !tp->dirty_rx && !tp->cur_rx) {
+	if ((tp->cp_cmd & PCIDAC) && !tp->cur_rx) {
 		void __iomem *ioaddr = tp->mmio_addr;
 
 		netif_info(tp, intr, dev, "disabling PCI DAC\n");
@@ -6035,10 +6034,8 @@ static int rtl_rx(struct net_device *dev, struct rtl8169_private *tp, u32 budget
 	unsigned int count;
 
 	cur_rx = tp->cur_rx;
-	rx_left = NUM_RX_DESC + tp->dirty_rx - cur_rx;
-	rx_left = min(rx_left, budget);
 
-	for (; rx_left > 0; rx_left--, cur_rx++) {
+	for (rx_left = min(budget, NUM_RX_DESC); rx_left > 0; rx_left--, cur_rx++) {
 		unsigned int entry = cur_rx % NUM_RX_DESC;
 		struct RxDesc *desc = tp->RxDescArray + entry;
 		u32 status;
@@ -6123,8 +6120,6 @@ process_pkt:
 	count = cur_rx - tp->cur_rx;
 	tp->cur_rx = cur_rx;
 
-	tp->dirty_rx += count;
-
 	return count;
 }
 
-- 
1.8.1

^ permalink raw reply related

* Re: [RFC(v2) net-next 12/13] ipv6: Remove temporary dst for icmpv6.
From: Cong Wang @ 2013-01-16  7:02 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F587E8.1060308@linux-ipv6.org>

On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
> ---
>   net/ipv6/ip6_fib.c |    2 +-
>   net/ipv6/route.c   |   98 ----------------------------------------------------
>   2 files changed, 1 insertion(+), 99 deletions(-)
[...]
> -
> -int icmp6_dst_gc(void)
> -{

Please remove its declaration too:
include/net/ip6_route.h:extern int icmp6_dst_gc(void);

^ permalink raw reply

* Re: [RFC(v2) net-next 13/13] ipv6: Complete neighbour entry removal from dst_entry.
From: Cong Wang @ 2013-01-16  7:09 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <50F587ED.5070602@linux-ipv6.org>

On 01/16/2013 12:46 AM, YOSHIFUJI Hideaki wrote:
> +#if 0
>   		if (rt->n && rt->n->dev == dev) {
>   			rt->n->dev = loopback_dev;
>   			dev_hold(loopback_dev);
>   			dev_put(dev);
>   		}
> +#endif

Why commenting this out instead of removing it? As rt->n is totally
removed, how possible could we reuse this code in future?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 1/3] net: stmmac: add gmac autoneg set for SGMII, TBI, and RTBI
From: Giuseppe CAVALLARO @ 2013-01-16  7:28 UTC (permalink / raw)
  To: Byungho An; +Cc: netdev, linux-kernel, davem, jeffrey.t.kirsher, kgene.kim
In-Reply-To: <00c001cdf369$9b25bdf0$d17139d0$@samsung.com>

Hello Byungho,

On 1/15/2013 10:45 PM, Byungho An wrote:
>
> This patch adds gmac autoneg set function for SGMII, TBI,
> or RTBI interface. In case of PHY's autoneg is set, gmac's
> autoneg enable bit should set. After checking phy's autoneg
> if phydev's autoneg is '1' gmac's ANE bit set for those
> interface.

Sorry I've some doubts about these patches.

Firstly if the MAC is able to manage RGMII/SGMII etc this should be 
verified by looking at the HW cap register: i.e. PCS bit.

   (I have no HW that support this so I cannot do any tests).

In case of this feature is actually supported then the driver could
manage everything bypassing the MDIO.
IMO, we could not need to call the stmmac_phy_init and we should not
use the PHYLIB.
I mean if we have the PCS module we could have a minimal support to get
link status, restart ANE etc w/o using at all the PHYLIB (so w/o 
touching the PHY regs via the MDIO/MDC).

   It could also be useful to complete the support with the RGMII... no
   extra effort should be needed while adding SGMII if you look at the
   core registers.
   If you add the RGMII on some platforms we could guarantee to manage
   the fix_mac_speed (see stmmac doc).

Looking at the other your patches, the ethtool support is not
completed. I expected to restart ANE, get/set link property etc.

Also pay attention to properly treat EEE. Maybe, as first stage we
should disable the feature in this case. We will see it later.
The question is that we could not be able to use some extra features
that currently need to dialog more with the PHY device. I mean what we
actually do by using PHYLIB.

>
> Signed-off-by: Byungho An <bh74.an@samsung.com>
> ---
>   drivers/net/ethernet/stmicro/stmmac/common.h         |    1 +
>   drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c |   11 +++++++++++
>   drivers/net/ethernet/stmicro/stmmac/stmmac_main.c    |    9 +++++++++
>   3 files changed, 21 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/common.h
> b/drivers/net/ethernet/stmicro/stmmac/common.h
> index 186d148..72ba769 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/common.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/common.h
> @@ -344,6 +344,7 @@ struct stmmac_ops {
>   	void (*reset_eee_mode) (void __iomem *ioaddr);
>   	void (*set_eee_timer) (void __iomem *ioaddr, int ls, int tw);
>   	void (*set_eee_pls) (void __iomem *ioaddr, int link);
> +	void (*set_autoneg) (void __iomem *ioaddr);
>   };
>
>   struct mac_link {
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> index bfe0226..a0737b39 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac1000_core.c
> @@ -297,6 +297,16 @@ static void  dwmac1000_set_eee_timer(void __iomem
> *ioaddr, int ls, int tw)
>   	writel(value, ioaddr + LPI_TIMER_CTRL);
>   }
>
> +static void dwmac1000_set_autoneg(void __iomem *ioaddr)
> +{
> +	u32 value;
> +
> +	value = readl(ioaddr + GMAC_AN_CTRL);
> +	value |= 0x1000;

pls use define instead of raw values ... see below.

> +	writel(value, ioaddr + GMAC_AN_CTRL);
> +}
> +
> +
>   static const struct stmmac_ops dwmac1000_ops = {
>   	.core_init = dwmac1000_core_init,
>   	.rx_ipc = dwmac1000_rx_ipc_enable,
> @@ -311,6 +321,7 @@ static const struct stmmac_ops dwmac1000_ops = {
>   	.reset_eee_mode =  dwmac1000_reset_eee_mode,
>   	.set_eee_timer =  dwmac1000_set_eee_timer,
>   	.set_eee_pls =  dwmac1000_set_eee_pls,
> +	.set_autoneg =  dwmac1000_set_autoneg,
>   };
>
>   struct mac_device_info *dwmac1000_setup(void __iomem *ioaddr)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index f07c061..3e28934 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1007,6 +1007,7 @@ static int stmmac_open(struct net_device *dev)
>   {
>   	struct stmmac_priv *priv = netdev_priv(dev);
>   	int ret;
> +	int interface = priv->plat->interface;
>
>   	clk_prepare_enable(priv->stmmac_clk);
>
> @@ -1041,6 +1042,14 @@ static int stmmac_open(struct net_device *dev)
>   	/* Initialize the MAC Core */
>   	priv->hw->mac->core_init(priv->ioaddr);
>
> +	/* If phy autoneg is on, set gmac autoneg for SGMII, TBI and RTBI*/
> +	if (interface == PHY_INTERFACE_MODE_SGMII ||
> +	    interface == PHY_INTERFACE_MODE_TBI ||
> +	    interface == PHY_INTERFACE_MODE_RTBI) {
> +		if (priv->phydev->autoneg)
> +			priv->hw->mac->set_autoneg(priv->ioaddr);
> +	}

we could use the following instead of priv->hw->mac->set_autoneg:

static void dwmac1000_ctrl_ane(void __iomem *ioaddr, bool restart)
{
     int value = GMAC_CTRL_ANE_EN;

     if (restart)
         value |= GMAC_CTRL_ANE_RESTART;

     writel(value, ioaddr +GMAC_AN_CTRL);
}

where we should defines all the missing macros for the registers 48, 49 ...

/* RGMI/SGMII defines */
#define GMAC_CTRL_ANE_SGMII_RAL (1 << 18)
#define GMAC_CTRL_ANE_EN       (1 << 12)
#define GMAC_CTRL_ANE_RESTART  (1 << 9)

The handler should store the link status that will be used to pass the 
info to the ethtool for example. We need to manage speed and duplex etc.

What happens if you run "ethtool eth0" command?
Or if you run mii-tool?
I expect to get the right speed and duplex at least.

Concerning the stmmac_set_pauseparam I'm also not sure you are doing the 
right thing. Note that you are not restarting the ANE at all ... (this 
is what the phy_start_aneg does). If set the bit 12 then you are 
enabling the ane. To restart it, you need to set the bit 9 in the reg 48.

I can support you on all the points above. Let me know.

BR,
peppe

> +
>   	/* Request the IRQ lines */
>   	ret = request_irq(dev->irq, stmmac_interrupt,
>   			 IRQF_SHARED, dev->name, dev);
>

^ permalink raw reply

* [Patch net-next 2/2] xfrm: replace rwlock on xfrm_km_list with rcu
From: Cong Wang @ 2013-01-16  8:05 UTC (permalink / raw)
  To: netdev; +Cc: Steffen Klassert, Herbert Xu, David S. Miller, Cong Wang
In-Reply-To: <1358323506-19571-1-git-send-email-amwang@redhat.com>

From: Cong Wang <amwang@redhat.com>


Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/xfrm/xfrm_state.c |   58 +++++++++++++++++++++++++-----------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index f567716..b40baf7 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -1647,27 +1647,26 @@ static void xfrm_replay_timer_handler(unsigned long data)
 }
 
 static LIST_HEAD(xfrm_km_list);
-static DEFINE_RWLOCK(xfrm_km_lock);
 
 void km_policy_notify(struct xfrm_policy *xp, int dir, const struct km_event *c)
 {
 	struct xfrm_mgr *km;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list)
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list)
 		if (km->notify_policy)
 			km->notify_policy(xp, dir, c);
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 }
 
 void km_state_notify(struct xfrm_state *x, const struct km_event *c)
 {
 	struct xfrm_mgr *km;
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list)
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list)
 		if (km->notify)
 			km->notify(x, c);
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 }
 
 EXPORT_SYMBOL(km_policy_notify);
@@ -1697,13 +1696,13 @@ int km_query(struct xfrm_state *x, struct xfrm_tmpl *t, struct xfrm_policy *pol)
 	int err = -EINVAL, acqret;
 	struct xfrm_mgr *km;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
 		acqret = km->acquire(x, t, pol);
 		if (!acqret)
 			err = acqret;
 	}
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 	return err;
 }
 EXPORT_SYMBOL(km_query);
@@ -1713,14 +1712,14 @@ int km_new_mapping(struct xfrm_state *x, xfrm_address_t *ipaddr, __be16 sport)
 	int err = -EINVAL;
 	struct xfrm_mgr *km;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
 		if (km->new_mapping)
 			err = km->new_mapping(x, ipaddr, sport);
 		if (!err)
 			break;
 	}
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 	return err;
 }
 EXPORT_SYMBOL(km_new_mapping);
@@ -1749,15 +1748,15 @@ int km_migrate(const struct xfrm_selector *sel, u8 dir, u8 type,
 	int ret;
 	struct xfrm_mgr *km;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
 		if (km->migrate) {
 			ret = km->migrate(sel, dir, type, m, num_migrate, k);
 			if (!ret)
 				err = ret;
 		}
 	}
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 	return err;
 }
 EXPORT_SYMBOL(km_migrate);
@@ -1769,15 +1768,15 @@ int km_report(struct net *net, u8 proto, struct xfrm_selector *sel, xfrm_address
 	int ret;
 	struct xfrm_mgr *km;
 
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
 		if (km->report) {
 			ret = km->report(net, proto, sel, addr);
 			if (!ret)
 				err = ret;
 		}
 	}
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 	return err;
 }
 EXPORT_SYMBOL(km_report);
@@ -1801,14 +1800,14 @@ int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen
 		goto out;
 
 	err = -EINVAL;
-	read_lock(&xfrm_km_lock);
-	list_for_each_entry(km, &xfrm_km_list, list) {
+	rcu_read_lock();
+	list_for_each_entry_rcu(km, &xfrm_km_list, list) {
 		pol = km->compile_policy(sk, optname, data,
 					 optlen, &err);
 		if (err >= 0)
 			break;
 	}
-	read_unlock(&xfrm_km_lock);
+	rcu_read_unlock();
 
 	if (err >= 0) {
 		xfrm_sk_policy_insert(sk, err, pol);
@@ -1822,20 +1821,23 @@ out:
 }
 EXPORT_SYMBOL(xfrm_user_policy);
 
+static DEFINE_SPINLOCK(xfrm_km_lock);
+
 int xfrm_register_km(struct xfrm_mgr *km)
 {
-	write_lock_bh(&xfrm_km_lock);
-	list_add_tail(&km->list, &xfrm_km_list);
-	write_unlock_bh(&xfrm_km_lock);
+	spin_lock_bh(&xfrm_km_lock);
+	list_add_tail_rcu(&km->list, &xfrm_km_list);
+	spin_unlock_bh(&xfrm_km_lock);
 	return 0;
 }
 EXPORT_SYMBOL(xfrm_register_km);
 
 int xfrm_unregister_km(struct xfrm_mgr *km)
 {
-	write_lock_bh(&xfrm_km_lock);
-	list_del(&km->list);
-	write_unlock_bh(&xfrm_km_lock);
+	spin_lock_bh(&xfrm_km_lock);
+	list_del_rcu(&km->list);
+	spin_unlock_bh(&xfrm_km_lock);
+	synchronize_rcu();
 	return 0;
 }
 EXPORT_SYMBOL(xfrm_unregister_km);
-- 
1.7.7.6

^ permalink raw reply related

* [Patch net-next 1/2] xfrm: replace rwlock on xfrm_state_afinfo with rcu
From: Cong Wang @ 2013-01-16  8:05 UTC (permalink / raw)
  To: netdev; +Cc: Steffen Klassert, Herbert Xu, David S. Miller, Cong Wang

From: Cong Wang <amwang@redhat.com>

Similar to commit 418a99ac6ad487dc9c42e6b0e85f941af56330f2
(Replace rwlock on xfrm_policy_afinfo with rcu), the rwlock
on xfrm_state_afinfo can be replaced by RCU too.

Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/xfrm/xfrm_state.c |   33 ++++++++++++++++-----------------
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 3459692..f567716 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -158,8 +158,8 @@ out_unlock:
 	mutex_unlock(&hash_resize_mutex);
 }
 
-static DEFINE_RWLOCK(xfrm_state_afinfo_lock);
-static struct xfrm_state_afinfo *xfrm_state_afinfo[NPROTO];
+static DEFINE_SPINLOCK(xfrm_state_afinfo_lock);
+static struct xfrm_state_afinfo __rcu *xfrm_state_afinfo[NPROTO];
 
 static DEFINE_SPINLOCK(xfrm_state_gc_lock);
 
@@ -173,17 +173,16 @@ static struct xfrm_state_afinfo *xfrm_state_lock_afinfo(unsigned int family)
 	struct xfrm_state_afinfo *afinfo;
 	if (unlikely(family >= NPROTO))
 		return NULL;
-	write_lock_bh(&xfrm_state_afinfo_lock);
+	spin_lock_bh(&xfrm_state_afinfo_lock);
 	afinfo = xfrm_state_afinfo[family];
 	if (unlikely(!afinfo))
-		write_unlock_bh(&xfrm_state_afinfo_lock);
+		spin_unlock_bh(&xfrm_state_afinfo_lock);
 	return afinfo;
 }
 
 static void xfrm_state_unlock_afinfo(struct xfrm_state_afinfo *afinfo)
-	__releases(xfrm_state_afinfo_lock)
 {
-	write_unlock_bh(&xfrm_state_afinfo_lock);
+	spin_unlock_bh(&xfrm_state_afinfo_lock);
 }
 
 int xfrm_register_type(const struct xfrm_type *type, unsigned short family)
@@ -1848,12 +1847,12 @@ int xfrm_state_register_afinfo(struct xfrm_state_afinfo *afinfo)
 		return -EINVAL;
 	if (unlikely(afinfo->family >= NPROTO))
 		return -EAFNOSUPPORT;
-	write_lock_bh(&xfrm_state_afinfo_lock);
+	spin_lock_bh(&xfrm_state_afinfo_lock);
 	if (unlikely(xfrm_state_afinfo[afinfo->family] != NULL))
 		err = -ENOBUFS;
 	else
-		xfrm_state_afinfo[afinfo->family] = afinfo;
-	write_unlock_bh(&xfrm_state_afinfo_lock);
+		rcu_assign_pointer(xfrm_state_afinfo[afinfo->family], afinfo);
+	spin_unlock_bh(&xfrm_state_afinfo_lock);
 	return err;
 }
 EXPORT_SYMBOL(xfrm_state_register_afinfo);
@@ -1865,14 +1864,15 @@ int xfrm_state_unregister_afinfo(struct xfrm_state_afinfo *afinfo)
 		return -EINVAL;
 	if (unlikely(afinfo->family >= NPROTO))
 		return -EAFNOSUPPORT;
-	write_lock_bh(&xfrm_state_afinfo_lock);
+	spin_lock_bh(&xfrm_state_afinfo_lock);
 	if (likely(xfrm_state_afinfo[afinfo->family] != NULL)) {
 		if (unlikely(xfrm_state_afinfo[afinfo->family] != afinfo))
 			err = -EINVAL;
 		else
-			xfrm_state_afinfo[afinfo->family] = NULL;
+			RCU_INIT_POINTER(xfrm_state_afinfo[afinfo->family], NULL);
 	}
-	write_unlock_bh(&xfrm_state_afinfo_lock);
+	spin_unlock_bh(&xfrm_state_afinfo_lock);
+	synchronize_rcu();
 	return err;
 }
 EXPORT_SYMBOL(xfrm_state_unregister_afinfo);
@@ -1882,17 +1882,16 @@ static struct xfrm_state_afinfo *xfrm_state_get_afinfo(unsigned int family)
 	struct xfrm_state_afinfo *afinfo;
 	if (unlikely(family >= NPROTO))
 		return NULL;
-	read_lock(&xfrm_state_afinfo_lock);
-	afinfo = xfrm_state_afinfo[family];
+	rcu_read_lock();
+	afinfo = rcu_dereference(xfrm_state_afinfo[family]);
 	if (unlikely(!afinfo))
-		read_unlock(&xfrm_state_afinfo_lock);
+		rcu_read_unlock();
 	return afinfo;
 }
 
 static void xfrm_state_put_afinfo(struct xfrm_state_afinfo *afinfo)
-	__releases(xfrm_state_afinfo_lock)
 {
-	read_unlock(&xfrm_state_afinfo_lock);
+	rcu_read_unlock();
 }
 
 /* Temporarily located here until net/xfrm/xfrm_tunnel.c is created */
-- 
1.7.7.6

^ permalink raw reply related

* Re: [PATCH net-next] bnx2x: fix GRO parameters
From: Yuval Mintz @ 2013-01-16  7:01 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Eilon Greenstein, ariele
In-Reply-To: <1358314662.19956.51.camel@edumazet-glaptop>

> -static u16 bnx2x_set_lro_mss(struct bnx2x *bp, u16 parsing_flags,
> -			     u16 len_on_bd)
> +static void bnx2x_set_gro_params(struct sk_buff *skb, struct bnx2x *bp,
> +				 u16 parsing_flags, u16 len_on_bd,
> +				 unsigned int pkt_len)

This is purely semantic, but our convention is for `struct bnx2x' to be
the first argument in our functions.

>  {
>  	/*
> -	 * TPA arrgregation won't have either IP options or TCP options
> +	 * TPA aggregation won't have either IP options or TCP options
>  	 * other than timestamp or IPv6 extension headers.
>  	 */
>  	u16 hdrs_len = ETH_HLEN + sizeof(struct tcphdr);

TPA_MODE_LRO indicates that an LRO aggregation was made by our FW. It seems
like your patch  eliminates the difference in configuration between the two
(GRO and LRO)

perhaps instead we should do something like:

+ static void bnx2x_set_lro_params(struct bnx2x *bp, struct sk_buff *skb,
+ 				   u16 parsing_flags, u16 len_on_bd,
+ 				   unsigned int pkt_len,
+				   bnx2x_tpa_mode_t mode)

And arrange its suggested code so that only gso_size will be set for LRO.

>  
>  	if (GET_FLAG(parsing_flags, PARSING_FLAGS_OVER_ETHERNET_PROTOCOL) ==
> -	    PRS_FLAG_OVERETH_IPV6)
> +	    PRS_FLAG_OVERETH_IPV6) {
>  		hdrs_len += sizeof(struct ipv6hdr);
> -	else /* IPv4 */
> +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> +	} else {
>  		hdrs_len += sizeof(struct iphdr);
> -
> +		skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> +	}
>  



>  #ifdef BNX2X_STOP_ON_ERROR
> @@ -651,7 +655,7 @@ static void bnx2x_gro_receive(struct bnx2x *bp, struct bnx2x_fastpath *fp,
>  			       struct sk_buff *skb)
>  {
>  #ifdef CONFIG_INET
> -	if (fp->mode == TPA_MODE_GRO && skb_shinfo(skb)->gso_size) {
> +	if (skb_shinfo(skb)->gso_size) {

This also seems like an incorrect removal, as TPA_MODE_LRO is (again)
a feasible option, and we wouldn't want the a `gro_complete' here.

>  		skb_set_network_header(skb, 0);
>  		switch (be16_to_cpu(skb->protocol)) {
>  		case ETH_P_IP:
> 

^ permalink raw reply

* Re: [PATCH v2 2/2] virtio-net: introduce a new control to set macaddr
From: Amos Kong @ 2013-01-16  8:24 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtualization, mst, kvm, netdev, qemu-devel, davem
In-Reply-To: <8243136.frWZnRkUJV@jason-thinkpad-t430s>

On Wed, Jan 16, 2013 at 02:20:39PM +0800, Jason Wang wrote:
> On Wednesday, January 16, 2013 01:57:01 PM akong@redhat.com wrote:
> > From: Amos Kong <akong@redhat.com>
> > 
> > Currently we write MAC address to pci config space byte by byte,
> > this means that we have an intermediate step where mac is wrong.
> > This patch introduced a new control command to set MAC address
> > in one time.
> > 
> > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
> > 
> > Signed-off-by: Amos Kong <akong@redhat.com>
> > ---
> >  drivers/net/virtio_net.c        | 24 +++++++++++++++++-------
> >  include/uapi/linux/virtio_net.h |  8 +++++++-
> >  2 files changed, 24 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 395ab4f..c8901b6 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -802,16 +802,25 @@ static int virtnet_set_mac_address(struct net_device
> > *dev, void *p) struct virtnet_info *vi = netdev_priv(dev);
> >  	struct virtio_device *vdev = vi->vdev;
> >  	int ret;
> > +	struct sockaddr *addr = p;
> > +	struct scatterlist sg;
> > 
> > -	ret = eth_mac_addr(dev, p);
> > -	if (ret)
> > -		return ret;
> > -
> > -	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
> > +	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> > +		sg_init_one(&sg, addr->sa_data, dev->addr_len);
> > +		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> > +					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
> > +					  &sg, 1, 0)) {
> > +			dev_warn(&vdev->dev,
> > +				 "Failed to set mac address by vq command.\n");
> > +			return -EINVAL;
> > +		}
> > +	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> >  		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
> > -		                  dev->dev_addr, dev->addr_len);
> > +				  addr->sa_data, dev->addr_len);
> > +	}
> > +	ret = eth_mac_addr(dev, p);
> > 
> 
> The you will the validity check in eth_mac_addr which may result a wrong mac 
> address to be set in the hardware (or is there any check in qemu) and a 
> inconsistency bettween what kernel assumes and qemu has.
> 
> You can take a look at netvsc driver that calls eth_mac_addr() first and 
> restore the software mac address when fail to enforce it to hardware.

Thanks for the catching, I will move eth_mac_addr() back to above,
just restore addr if fail to send command.

I will also use DEFINE_PROP_BIT to fix migration issue, thanks.
 
> Thanks
> > -	return 0;
> > +	return ret;
> >  }



^ permalink raw reply

* Re: [PATCH v2 2/2] virtio-net: introduce a new control to set macaddr
From: Michael S. Tsirkin @ 2013-01-16  8:36 UTC (permalink / raw)
  To: Amos Kong; +Cc: kvm, netdev, qemu-devel, virtualization, davem
In-Reply-To: <20130116082447.GA31074@t430s.nay.redhat.com>

On Wed, Jan 16, 2013 at 04:24:47PM +0800, Amos Kong wrote:
> On Wed, Jan 16, 2013 at 02:20:39PM +0800, Jason Wang wrote:
> > On Wednesday, January 16, 2013 01:57:01 PM akong@redhat.com wrote:
> > > From: Amos Kong <akong@redhat.com>
> > > 
> > > Currently we write MAC address to pci config space byte by byte,
> > > this means that we have an intermediate step where mac is wrong.
> > > This patch introduced a new control command to set MAC address
> > > in one time.
> > > 
> > > VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
> > > 
> > > Signed-off-by: Amos Kong <akong@redhat.com>
> > > ---
> > >  drivers/net/virtio_net.c        | 24 +++++++++++++++++-------
> > >  include/uapi/linux/virtio_net.h |  8 +++++++-
> > >  2 files changed, 24 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > > index 395ab4f..c8901b6 100644
> > > --- a/drivers/net/virtio_net.c
> > > +++ b/drivers/net/virtio_net.c
> > > @@ -802,16 +802,25 @@ static int virtnet_set_mac_address(struct net_device
> > > *dev, void *p) struct virtnet_info *vi = netdev_priv(dev);
> > >  	struct virtio_device *vdev = vi->vdev;
> > >  	int ret;
> > > +	struct sockaddr *addr = p;
> > > +	struct scatterlist sg;
> > > 
> > > -	ret = eth_mac_addr(dev, p);
> > > -	if (ret)
> > > -		return ret;
> > > -
> > > -	if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
> > > +	if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
> > > +		sg_init_one(&sg, addr->sa_data, dev->addr_len);
> > > +		if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
> > > +					  VIRTIO_NET_CTRL_MAC_ADDR_SET,
> > > +					  &sg, 1, 0)) {
> > > +			dev_warn(&vdev->dev,
> > > +				 "Failed to set mac address by vq command.\n");
> > > +			return -EINVAL;
> > > +		}
> > > +	} else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> > >  		vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
> > > -		                  dev->dev_addr, dev->addr_len);
> > > +				  addr->sa_data, dev->addr_len);
> > > +	}
> > > +	ret = eth_mac_addr(dev, p);
> > > 
> > 
> > The you will the validity check in eth_mac_addr which may result a wrong mac 
> > address to be set in the hardware (or is there any check in qemu) and a 
> > inconsistency bettween what kernel assumes and qemu has.
> > 
> > You can take a look at netvsc driver that calls eth_mac_addr() first and 
> > restore the software mac address when fail to enforce it to hardware.
> 
> Thanks for the catching, I will move eth_mac_addr() back to above,
> just restore addr if fail to send command.
> 
> I will also use DEFINE_PROP_BIT to fix migration issue, thanks.

And clear it if running with a compat machine type.

> > Thanks
> > > -	return 0;
> > > +	return ret;
> > >  }
> 

^ permalink raw reply

* Re: [PATCH] ipv4: Remove output route check in ipv4_mtu
From: Julian Anastasov @ 2013-01-16  8:58 UTC (permalink / raw)
  To: Steffen Klassert; +Cc: David Miller, timo.teras, luky-37, pupilla, netdev
In-Reply-To: <20130116063645.GC18940@secunet.com>


	Hello,

On Wed, 16 Jan 2013, Steffen Klassert wrote:

> The output route check was introduced with git commit 261663b0
> (ipv4: Don't use the cached pmtu informations for input routes)
> during times when we cached the pmtu informations on the
> inetpeer. Now the pmtu informations are back in the routes,
> so this check is obsolete. It also had some unwanted side effects,
> as reported by Timo Teras and Lukas Tribus.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> ---
>  net/ipv4/route.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 844a9ef..6e4a89c 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1120,7 +1120,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
>  	if (!mtu || time_after_eq(jiffies, rt->dst.expires))
>  		mtu = dst_metric_raw(dst, RTAX_MTU);
>  
> -	if (mtu && rt_is_output_route(rt))
> +	if (mtu)
>  		return mtu;

	This fix looks good to me. But I see that we
have another problem here. doc/ip-cref.tex in iproute2
claims that a locked MTU value has priority and
PMTU should not be considered.

	IIRC, rt_pmtu is valid only for output routes but
we do not check the lock flag here. What about such
variant:

static unsigned int ipv4_mtu(const struct dst_entry *dst)
{
	const struct rtable *rt = (const struct rtable *) dst;
	unsigned int mtu = rt->rt_pmtu;

	if (unlikely(dst_metric_locked(dst, RTAX_MTU))) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu) {
			mtu = dst->dev->mtu;
			if (rt->rt_uses_gateway && mtu > 576)
				mtu = 576;
		}
	} else if (!mtu || time_after_eq(jiffies, rt->dst.expires)) {
		mtu = dst_metric_raw(dst, RTAX_MTU);
		if (!mtu)
			mtu = dst->dev->mtu;
	}
	if (mtu > IP_MAX_MTU)
		mtu = IP_MAX_MTU;

	return mtu;
}

	I.e. order becomes:

mtu lock non-zero => fixed fib_mtu
mtu lock 0 => device MTU, up to 576 if via GW, ignore PMTU
PMTU (output routes)
mtu non-zero => fib_mtu
device MTU

	Also, it seems __ip_rt_update_pmtu should start
with such dst_metric_locked(dst, RTAX_MTU) check?

>  	mtu = dst->dev->mtu;
> -- 
> 1.7.9.5

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* [net 0/2][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-01-16  9:03 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series contains updates to ixgbe only.

The following are changes since commit daf3ec688e057f6060fb9bb0819feac7a8bbf45c:
  tg3: Fix crc errors on jumbo frame receive
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Jacob Keller (2):
  ixgbe: only compile ixgbe_debugfs.o when enabled
  ixgbe: Fix overwriting of rx_mtrl in ixgbe_ptp_hwtstamp_ioctl

 drivers/net/ethernet/intel/ixgbe/Makefile        | 3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 5 -----
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c     | 4 ++--
 3 files changed, 4 insertions(+), 8 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [net 1/2] ixgbe: only compile ixgbe_debugfs.o when enabled
From: Jeff Kirsher @ 2013-01-16  9:03 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327029-13340-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

This patch modifies ixgbe_debugfs.c and the Makefile for the ixgbe
driver to only compile the file when the config is enabled. This means
we can remove the #ifdef inside the ixgbe_debugfs.c file.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/Makefile        | 3 ++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c | 5 -----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/Makefile b/drivers/net/ethernet/intel/ixgbe/Makefile
index f3a632b..687c83d 100644
--- a/drivers/net/ethernet/intel/ixgbe/Makefile
+++ b/drivers/net/ethernet/intel/ixgbe/Makefile
@@ -32,7 +32,7 @@
 
 obj-$(CONFIG_IXGBE) += ixgbe.o
 
-ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o ixgbe_debugfs.o\
+ixgbe-objs := ixgbe_main.o ixgbe_common.o ixgbe_ethtool.o \
               ixgbe_82599.o ixgbe_82598.o ixgbe_phy.o ixgbe_sriov.o \
               ixgbe_mbx.o ixgbe_x540.o ixgbe_lib.o ixgbe_ptp.o
 
@@ -40,4 +40,5 @@ ixgbe-$(CONFIG_IXGBE_DCB) +=  ixgbe_dcb.o ixgbe_dcb_82598.o \
                               ixgbe_dcb_82599.o ixgbe_dcb_nl.o
 
 ixgbe-$(CONFIG_IXGBE_HWMON) += ixgbe_sysfs.o
+ixgbe-$(CONFIG_DEBUG_FS) += ixgbe_debugfs.o
 ixgbe-$(CONFIG_FCOE:m=y) += ixgbe_fcoe.o
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index 50aa546..3504686 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -24,9 +24,6 @@
   Intel Corporation, 5200 N.E. Elam Young Parkway, Hillsboro, OR 97124-6497
 
 *******************************************************************************/
-
-#ifdef CONFIG_DEBUG_FS
-
 #include <linux/debugfs.h>
 #include <linux/module.h>
 
@@ -277,5 +274,3 @@ void ixgbe_dbg_exit(void)
 {
 	debugfs_remove_recursive(ixgbe_dbg_root);
 }
-
-#endif /* CONFIG_DEBUG_FS */
-- 
1.7.11.7

^ permalink raw reply related

* [net 2/2] ixgbe: Fix overwriting of rx_mtrl in ixgbe_ptp_hwtstamp_ioctl
From: Jeff Kirsher @ 2013-01-16  9:03 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327029-13340-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

This patch corrects a bug introduced by commit f3444d8b. The rxmtrl value for
the UDP port to timestamp on was moved above the switch statement, but was
overwritten to 0 if the ioctl selected one of the V1 filters.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Phil Schmitt <phillip.j.schmitt@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
index 1a751c9..bb9256a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ptp.c
@@ -660,11 +660,11 @@ int ixgbe_ptp_hwtstamp_ioctl(struct ixgbe_adapter *adapter,
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
 		tsync_rx_ctl |= IXGBE_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_mtrl = IXGBE_RXMTRL_V1_SYNC_MSG;
+		tsync_rx_mtrl |= IXGBE_RXMTRL_V1_SYNC_MSG;
 		break;
 	case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
 		tsync_rx_ctl |= IXGBE_TSYNCRXCTL_TYPE_L4_V1;
-		tsync_rx_mtrl = IXGBE_RXMTRL_V1_DELAY_REQ_MSG;
+		tsync_rx_mtrl |= IXGBE_RXMTRL_V1_DELAY_REQ_MSG;
 		break;
 	case HWTSTAMP_FILTER_PTP_V2_EVENT:
 	case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
-- 
1.7.11.7

^ permalink raw reply related

* [net-nexti v2 00/15][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2013-01-16  9:10 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, gospo, sassmann

This series contains updates to e1000e only.

v2- updates patch 09/15 "e1000e: resolve checkpatch PREFER_PR_LEVEL warning"
    based on feedback from Joe Perches.

The following are changes since commit 247fa82be1301e4f89eaa5dc10be62a9cc84e8c5:
  bnx2x: Introduce 2013 and advance version to 1.78.02
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next master

Bruce Allan (15):
  e1000e: SerDes autoneg flow control
  e1000e: Acquire/release semaphore when writing each EEPROM page
  e1000e: Invalid Image CSUM bit changed for I217
  e1000e: helper functions for accessing EMI registers
  e1000e: 82577: workaround for link drop issue
  e1000e: fix enabling of EEE on 82579 and I217
  e1000e: unexpected "Reset adapter" message when cable pulled
  e1000e: add missing bailout on error
  e1000e: resolve checkpatch PREFER_PR_LEVEL warning
  e1000e: cleanup redundant statistics counter
  e1000e: cleanup unusual comment placement
  e1000e: cleanup unnecessary line wrap
  e1000e: cleanup magic number
  e1000e: cleanup code duplication
  e1000e: merge multiple conditional statements into one

 drivers/net/ethernet/intel/e1000e/defines.h |  10 +-
 drivers/net/ethernet/intel/e1000e/e1000.h   |   2 +-
 drivers/net/ethernet/intel/e1000e/ethtool.c |   1 -
 drivers/net/ethernet/intel/e1000e/hw.h      |   4 +
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 213 +++++++++++++++++++++-------
 drivers/net/ethernet/intel/e1000e/mac.c     | 125 ++++++++++++++++
 drivers/net/ethernet/intel/e1000e/netdev.c  |  66 ++++-----
 drivers/net/ethernet/intel/e1000e/nvm.c     |  26 ++--
 drivers/net/ethernet/intel/e1000e/param.c   |   3 +-
 9 files changed, 339 insertions(+), 111 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [net-next 01/15] e1000e: SerDes autoneg flow control
From: Jeff Kirsher @ 2013-01-16  9:10 UTC (permalink / raw)
  To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

Enables flow control to be set in SerDes autoneg mode. This is what is
done for copper, but relies on a different set of register/bit checks
since this is all done within the Mac registers.

Remove inapplicable comment in defines.h

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/defines.h |   6 +-
 drivers/net/ethernet/intel/e1000e/hw.h      |   4 +
 drivers/net/ethernet/intel/e1000e/mac.c     | 125 ++++++++++++++++++++++++++++
 3 files changed, 132 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 02a12b6..2073532 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -241,9 +241,9 @@
 #define E1000_CTRL_VME      0x40000000  /* IEEE VLAN mode enable */
 #define E1000_CTRL_PHY_RST  0x80000000  /* PHY Reset */
 
-/* Bit definitions for the Management Data IO (MDIO) and Management Data
- * Clock (MDC) pins in the Device Control Register.
- */
+#define E1000_PCS_LCTL_FORCE_FCTRL	0x80
+
+#define E1000_PCS_LSTS_AN_COMPLETE	0x10000
 
 /* Device Status */
 #define E1000_STATUS_FD         0x00000001      /* Full duplex.0=half,1=full */
diff --git a/drivers/net/ethernet/intel/e1000e/hw.h b/drivers/net/ethernet/intel/e1000e/hw.h
index cf21777..06239fe 100644
--- a/drivers/net/ethernet/intel/e1000e/hw.h
+++ b/drivers/net/ethernet/intel/e1000e/hw.h
@@ -191,6 +191,10 @@ enum e1e_registers {
 	E1000_ICTXQMTC = 0x0411C, /* Irq Cause Tx Queue MinThreshold Count */
 	E1000_ICRXDMTC = 0x04120, /* Irq Cause Rx Desc MinThreshold Count */
 	E1000_ICRXOC   = 0x04124, /* Irq Cause Receiver Overrun Count */
+	E1000_PCS_LCTL = 0x04208, /* PCS Link Control - RW */
+	E1000_PCS_LSTAT = 0x0420C, /* PCS Link Status - RO */
+	E1000_PCS_ANADV = 0x04218, /* AN advertisement - RW */
+	E1000_PCS_LPAB = 0x0421C, /* Link Partner Ability - RW */
 	E1000_RXCSUM   = 0x05000, /* Rx Checksum Control - RW */
 	E1000_RFCTL    = 0x05008, /* Receive Filter Control */
 	E1000_MTA      = 0x05200, /* Multicast Table Array - RW Array */
diff --git a/drivers/net/ethernet/intel/e1000e/mac.c b/drivers/net/ethernet/intel/e1000e/mac.c
index 54d9daf..0fa4c06 100644
--- a/drivers/net/ethernet/intel/e1000e/mac.c
+++ b/drivers/net/ethernet/intel/e1000e/mac.c
@@ -1021,6 +1021,7 @@ s32 e1000e_config_fc_after_link_up(struct e1000_hw *hw)
 {
 	struct e1000_mac_info *mac = &hw->mac;
 	s32 ret_val = 0;
+	u32 pcs_status_reg, pcs_adv_reg, pcs_lp_ability_reg, pcs_ctrl_reg;
 	u16 mii_status_reg, mii_nway_adv_reg, mii_nway_lp_ability_reg;
 	u16 speed, duplex;
 
@@ -1185,6 +1186,130 @@ s32 e1000e_config_fc_after_link_up(struct e1000_hw *hw)
 		}
 	}
 
+	/* Check for the case where we have SerDes media and auto-neg is
+	 * enabled.  In this case, we need to check and see if Auto-Neg
+	 * has completed, and if so, how the PHY and link partner has
+	 * flow control configured.
+	 */
+	if ((hw->phy.media_type == e1000_media_type_internal_serdes) &&
+	    mac->autoneg) {
+		/* Read the PCS_LSTS and check to see if AutoNeg
+		 * has completed.
+		 */
+		pcs_status_reg = er32(PCS_LSTAT);
+
+		if (!(pcs_status_reg & E1000_PCS_LSTS_AN_COMPLETE)) {
+			e_dbg("PCS Auto Neg has not completed.\n");
+			return ret_val;
+		}
+
+		/* The AutoNeg process has completed, so we now need to
+		 * read both the Auto Negotiation Advertisement
+		 * Register (PCS_ANADV) and the Auto_Negotiation Base
+		 * Page Ability Register (PCS_LPAB) to determine how
+		 * flow control was negotiated.
+		 */
+		pcs_adv_reg = er32(PCS_ANADV);
+		pcs_lp_ability_reg = er32(PCS_LPAB);
+
+		/* Two bits in the Auto Negotiation Advertisement Register
+		 * (PCS_ANADV) and two bits in the Auto Negotiation Base
+		 * Page Ability Register (PCS_LPAB) determine flow control
+		 * for both the PHY and the link partner.  The following
+		 * table, taken out of the IEEE 802.3ab/D6.0 dated March 25,
+		 * 1999, describes these PAUSE resolution bits and how flow
+		 * control is determined based upon these settings.
+		 * NOTE:  DC = Don't Care
+		 *
+		 *   LOCAL DEVICE  |   LINK PARTNER
+		 * PAUSE | ASM_DIR | PAUSE | ASM_DIR | NIC Resolution
+		 *-------|---------|-------|---------|--------------------
+		 *   0   |    0    |  DC   |   DC    | e1000_fc_none
+		 *   0   |    1    |   0   |   DC    | e1000_fc_none
+		 *   0   |    1    |   1   |    0    | e1000_fc_none
+		 *   0   |    1    |   1   |    1    | e1000_fc_tx_pause
+		 *   1   |    0    |   0   |   DC    | e1000_fc_none
+		 *   1   |   DC    |   1   |   DC    | e1000_fc_full
+		 *   1   |    1    |   0   |    0    | e1000_fc_none
+		 *   1   |    1    |   0   |    1    | e1000_fc_rx_pause
+		 *
+		 * Are both PAUSE bits set to 1?  If so, this implies
+		 * Symmetric Flow Control is enabled at both ends.  The
+		 * ASM_DIR bits are irrelevant per the spec.
+		 *
+		 * For Symmetric Flow Control:
+		 *
+		 *   LOCAL DEVICE  |   LINK PARTNER
+		 * PAUSE | ASM_DIR | PAUSE | ASM_DIR | Result
+		 *-------|---------|-------|---------|--------------------
+		 *   1   |   DC    |   1   |   DC    | e1000_fc_full
+		 *
+		 */
+		if ((pcs_adv_reg & E1000_TXCW_PAUSE) &&
+		    (pcs_lp_ability_reg & E1000_TXCW_PAUSE)) {
+			/* Now we need to check if the user selected Rx ONLY
+			 * of pause frames.  In this case, we had to advertise
+			 * FULL flow control because we could not advertise Rx
+			 * ONLY. Hence, we must now check to see if we need to
+			 * turn OFF the TRANSMISSION of PAUSE frames.
+			 */
+			if (hw->fc.requested_mode == e1000_fc_full) {
+				hw->fc.current_mode = e1000_fc_full;
+				e_dbg("Flow Control = FULL.\n");
+			} else {
+				hw->fc.current_mode = e1000_fc_rx_pause;
+				e_dbg("Flow Control = Rx PAUSE frames only.\n");
+			}
+		}
+		/* For receiving PAUSE frames ONLY.
+		 *
+		 *   LOCAL DEVICE  |   LINK PARTNER
+		 * PAUSE | ASM_DIR | PAUSE | ASM_DIR | Result
+		 *-------|---------|-------|---------|--------------------
+		 *   0   |    1    |   1   |    1    | e1000_fc_tx_pause
+		 */
+		else if (!(pcs_adv_reg & E1000_TXCW_PAUSE) &&
+			 (pcs_adv_reg & E1000_TXCW_ASM_DIR) &&
+			 (pcs_lp_ability_reg & E1000_TXCW_PAUSE) &&
+			 (pcs_lp_ability_reg & E1000_TXCW_ASM_DIR)) {
+			hw->fc.current_mode = e1000_fc_tx_pause;
+			e_dbg("Flow Control = Tx PAUSE frames only.\n");
+		}
+		/* For transmitting PAUSE frames ONLY.
+		 *
+		 *   LOCAL DEVICE  |   LINK PARTNER
+		 * PAUSE | ASM_DIR | PAUSE | ASM_DIR | Result
+		 *-------|---------|-------|---------|--------------------
+		 *   1   |    1    |   0   |    1    | e1000_fc_rx_pause
+		 */
+		else if ((pcs_adv_reg & E1000_TXCW_PAUSE) &&
+			 (pcs_adv_reg & E1000_TXCW_ASM_DIR) &&
+			 !(pcs_lp_ability_reg & E1000_TXCW_PAUSE) &&
+			 (pcs_lp_ability_reg & E1000_TXCW_ASM_DIR)) {
+			hw->fc.current_mode = e1000_fc_rx_pause;
+			e_dbg("Flow Control = Rx PAUSE frames only.\n");
+		} else {
+			/* Per the IEEE spec, at this point flow control
+			 * should be disabled.
+			 */
+			hw->fc.current_mode = e1000_fc_none;
+			e_dbg("Flow Control = NONE.\n");
+		}
+
+		/* Now we call a subroutine to actually force the MAC
+		 * controller to use the correct flow control settings.
+		 */
+		pcs_ctrl_reg = er32(PCS_LCTL);
+		pcs_ctrl_reg |= E1000_PCS_LCTL_FORCE_FCTRL;
+		ew32(PCS_LCTL, pcs_ctrl_reg);
+
+		ret_val = e1000e_force_mac_fc(hw);
+		if (ret_val) {
+			e_dbg("Error forcing flow control settings\n");
+			return ret_val;
+		}
+	}
+
 	return 0;
 }
 
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 02/15] e1000e: Acquire/release semaphore when writing each EEPROM page
From: Jeff Kirsher @ 2013-01-16  9:10 UTC (permalink / raw)
  To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

When data blocks are written to the EEPROM, the HW/SW/FW semaphore must be
held for the duration.  With large data blocks on 80003es2lan, 82571 and
82572, this can take too long and cause the firmware to take ownership of
the semaphore and consequently ownership of writes to the EEPROM.
Instead, acquire and release the semaphore for each page of the block
written.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/nvm.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/nvm.c b/drivers/net/ethernet/intel/e1000e/nvm.c
index b646880..71938ed 100644
--- a/drivers/net/ethernet/intel/e1000e/nvm.c
+++ b/drivers/net/ethernet/intel/e1000e/nvm.c
@@ -359,7 +359,7 @@ s32 e1000e_read_nvm_eerd(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
 s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
 {
 	struct e1000_nvm_info *nvm = &hw->nvm;
-	s32 ret_val;
+	s32 ret_val = -E1000_ERR_NVM;
 	u16 widx = 0;
 
 	/* A check for invalid values:  offset too large, too many words,
@@ -371,16 +371,18 @@ s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
 		return -E1000_ERR_NVM;
 	}
 
-	ret_val = nvm->ops.acquire(hw);
-	if (ret_val)
-		return ret_val;
-
 	while (widx < words) {
 		u8 write_opcode = NVM_WRITE_OPCODE_SPI;
 
-		ret_val = e1000_ready_nvm_eeprom(hw);
+		ret_val = nvm->ops.acquire(hw);
 		if (ret_val)
-			goto release;
+			return ret_val;
+
+		ret_val = e1000_ready_nvm_eeprom(hw);
+		if (ret_val) {
+			nvm->ops.release(hw);
+			return ret_val;
+		}
 
 		e1000_standby_nvm(hw);
 
@@ -413,12 +415,10 @@ s32 e1000e_write_nvm_spi(struct e1000_hw *hw, u16 offset, u16 words, u16 *data)
 				break;
 			}
 		}
+		usleep_range(10000, 20000);
+		nvm->ops.release(hw);
 	}
 
-	usleep_range(10000, 20000);
-release:
-	nvm->ops.release(hw);
-
 	return ret_val;
 }
 
-- 
1.7.11.7

^ permalink raw reply related

* [net-next 03/15] e1000e: Invalid Image CSUM bit changed for I217
From: Jeff Kirsher @ 2013-01-16  9:10 UTC (permalink / raw)
  To: davem; +Cc: Bruce Allan, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1358327427-3146-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Bruce Allan <bruce.w.allan@intel.com>

On I217, the bit that indicates an invalid EEPROM (NVM) image checksum has
changed from previous ICH/PCH LOMs.  When validating the EEPROM checksum,
check the appropriate bit on different devices.

Signed-off-by: Bruce Allan <bruce.w.allan@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/e1000e/defines.h |  4 ++++
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 29 +++++++++++++++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
index 2073532..7326ea2 100644
--- a/drivers/net/ethernet/intel/e1000e/defines.h
+++ b/drivers/net/ethernet/intel/e1000e/defines.h
@@ -639,6 +639,10 @@
 /* NVM Word Offsets */
 #define NVM_COMPAT                 0x0003
 #define NVM_ID_LED_SETTINGS        0x0004
+#define NVM_FUTURE_INIT_WORD1      0x0019
+#define NVM_COMPAT_VALID_CSUM      0x0001
+#define NVM_FUTURE_INIT_WORD1_VALID_CSUM	0x0040
+
 #define NVM_INIT_CONTROL2_REG      0x000F
 #define NVM_INIT_CONTROL3_PORT_B   0x0014
 #define NVM_INIT_3GIO_3            0x001A
diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index 9763365..7d5f6b7 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -2949,19 +2949,32 @@ static s32 e1000_validate_nvm_checksum_ich8lan(struct e1000_hw *hw)
 {
 	s32 ret_val;
 	u16 data;
+	u16 word;
+	u16 valid_csum_mask;
 
-	/* Read 0x19 and check bit 6.  If this bit is 0, the checksum
-	 * needs to be fixed.  This bit is an indication that the NVM
-	 * was prepared by OEM software and did not calculate the
-	 * checksum...a likely scenario.
+	/* Read NVM and check Invalid Image CSUM bit.  If this bit is 0,
+	 * the checksum needs to be fixed.  This bit is an indication that
+	 * the NVM was prepared by OEM software and did not calculate
+	 * the checksum...a likely scenario.
 	 */
-	ret_val = e1000_read_nvm(hw, 0x19, 1, &data);
+	switch (hw->mac.type) {
+	case e1000_pch_lpt:
+		word = NVM_COMPAT;
+		valid_csum_mask = NVM_COMPAT_VALID_CSUM;
+		break;
+	default:
+		word = NVM_FUTURE_INIT_WORD1;
+		valid_csum_mask = NVM_FUTURE_INIT_WORD1_VALID_CSUM;
+		break;
+	}
+
+	ret_val = e1000_read_nvm(hw, word, 1, &data);
 	if (ret_val)
 		return ret_val;
 
-	if (!(data & 0x40)) {
-		data |= 0x40;
-		ret_val = e1000_write_nvm(hw, 0x19, 1, &data);
+	if (!(data & valid_csum_mask)) {
+		data |= valid_csum_mask;
+		ret_val = e1000_write_nvm(hw, word, 1, &data);
 		if (ret_val)
 			return ret_val;
 		ret_val = e1000e_update_nvm_checksum(hw);
-- 
1.7.11.7

^ 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