Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH, REVERT] Re: [forcedeth bug] Re: [GIT] Networking
From: David Miller @ 2011-08-05 10:19 UTC (permalink / raw)
  To: mingo; +Cc: jpirko, torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20110805101625.GA11502@elte.hu>

From: Ingo Molnar <mingo@elte.hu>
Date: Fri, 5 Aug 2011 12:16:25 +0200

> I have reverted the two forcedeth commits:
> 
>   0891b0e08937: forcedeth: fix vlans
>   3326c784c9f4: forcedeth: do vlan cleanup
> 
> and also reverted two vlan commits that the pre-cleanup driver 
> depended on:
> 
>   ffcf9b767293: vlan: kill vlan_gro_frags and vlan_gro_receive
>   7890a5b9cbfd: vlan: kill ndo_vlan_rx_register
> 
> and this finally gave me a working forcedeth driver. I've attached 
> the working revert below.

Jiri please diagnose this immediately otherwise I will have to apply
Ingo's reverts.

Ingo has been reporting this regression for days and you haven't said
anything.  That's not acceptable.

^ permalink raw reply

* [PATCH, REVERT] Re: [forcedeth bug] Re: [GIT] Networking
From: Ingo Molnar @ 2011-08-05 10:16 UTC (permalink / raw)
  To: David Miller, Jiri Pirko; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <20110804215354.GA7056@elte.hu>


* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Ingo Molnar <mingo@elte.hu> wrote:
> 
> >  0891b0e08937: forcedeth: fix vlans
> 
> Hm, forcedeth is still giving me trouble even on latest -git that has 
> the above fix included.
> 
> The symptom is a stuck interface, no packets in. There's a frame 
> error RX packet:
> 
>  [root@mercury ~]# ifconfig eth0
>  eth0      Link encap:Ethernet  HWaddr 00:13:D4:DC:41:12  
>            inet addr:10.0.1.13  Bcast:10.0.1.255  Mask:255.255.255.0
>            UP BROADCAST RUNNING MULTICAST  MTU:1500  Metric:1
>            RX packets:0 errors:1 dropped:0 overruns:0 frame:1
>            TX packets:531 errors:0 dropped:0 overruns:0 carrier:0
>            collisions:0 txqueuelen:1000 
>            RX bytes:0 (0.0 b)  TX bytes:34112 (33.3 KiB)
>            Interrupt:35 
> 
> Weirdly enough a defconfig x86 bootup works just fine - it's certain 
> .config combinations that trigger the bug. I've attached such a 
> config.
> 
> Note that at least once i've observed a seemingly good kernel going 
> 'bad' after a couple of minutes uptime. I've also observed 
> intermittent behavior - apparent lost packets and a laggy network.
> 
> I have done 3 failed attempts to bisect it any further - i got to the 
> commit that got fixed by:
> 
>   0891b0e08937: forcedeth: fix vlans
> 
> ... but that's something we already knew.
> 
> Let me know if there's any data i can provide to help debug this 
> problem.

I have reverted the two forcedeth commits:

  0891b0e08937: forcedeth: fix vlans
  3326c784c9f4: forcedeth: do vlan cleanup

and also reverted two vlan commits that the pre-cleanup driver 
depended on:

  ffcf9b767293: vlan: kill vlan_gro_frags and vlan_gro_receive
  7890a5b9cbfd: vlan: kill ndo_vlan_rx_register

and this finally gave me a working forcedeth driver. I've attached 
the working revert below.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/drivers/net/forcedeth.c b/drivers/net/forcedeth.c
index e55df30..537b695 100644
--- a/drivers/net/forcedeth.c
+++ b/drivers/net/forcedeth.c
@@ -820,6 +820,9 @@ struct fe_priv {
 	struct nv_skb_map *tx_end_flip;
 	int tx_stop;
 
+	/* vlan fields */
+	struct vlan_group *vlangrp;
+
 	/* msi/msi-x fields */
 	u32 msi_flags;
 	struct msix_entry msi_x_entry[NV_MSI_X_MAX_VECTORS];
@@ -2763,20 +2766,17 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
 			skb->protocol = eth_type_trans(skb, dev);
 			prefetch(skb->data);
 
-			vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
-
-			/*
-			 * There's need to check for NETIF_F_HW_VLAN_RX here.
-			 * Even if vlan rx accel is disabled,
-			 * NV_RX3_VLAN_TAG_PRESENT is pseudo randomly set.
-			 */
-			if (dev->features & NETIF_F_HW_VLAN_RX &&
-			    vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
-				u16 vid = vlanflags & NV_RX3_VLAN_TAG_MASK;
-
-				__vlan_hwaccel_put_tag(skb, vid);
+			if (likely(!np->vlangrp)) {
+				napi_gro_receive(&np->napi, skb);
+			} else {
+				vlanflags = le32_to_cpu(np->get_rx.ex->buflow);
+				if (vlanflags & NV_RX3_VLAN_TAG_PRESENT) {
+					vlan_gro_receive(&np->napi, np->vlangrp,
+							 vlanflags & NV_RX3_VLAN_TAG_MASK, skb);
+				} else {
+					napi_gro_receive(&np->napi, skb);
+				}
 			}
-			napi_gro_receive(&np->napi, skb);
 
 			dev->stats.rx_packets++;
 			dev->stats.rx_bytes += len;
@@ -4484,27 +4484,6 @@ static u32 nv_fix_features(struct net_device *dev, u32 features)
 	return features;
 }
 
-static void nv_vlan_mode(struct net_device *dev, u32 features)
-{
-	struct fe_priv *np = get_nvpriv(dev);
-
-	spin_lock_irq(&np->lock);
-
-	if (features & NETIF_F_HW_VLAN_RX)
-		np->txrxctl_bits |= NVREG_TXRXCTL_VLANSTRIP;
-	else
-		np->txrxctl_bits &= ~NVREG_TXRXCTL_VLANSTRIP;
-
-	if (features & NETIF_F_HW_VLAN_TX)
-		np->txrxctl_bits |= NVREG_TXRXCTL_VLANINS;
-	else
-		np->txrxctl_bits &= ~NVREG_TXRXCTL_VLANINS;
-
-	writel(np->txrxctl_bits, get_hwbase(dev) + NvRegTxRxControl);
-
-	spin_unlock_irq(&np->lock);
-}
-
 static int nv_set_features(struct net_device *dev, u32 features)
 {
 	struct fe_priv *np = netdev_priv(dev);
@@ -4525,9 +4504,6 @@ static int nv_set_features(struct net_device *dev, u32 features)
 		spin_unlock_irq(&np->lock);
 	}
 
-	if (changed & (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX))
-		nv_vlan_mode(dev, features);
-
 	return 0;
 }
 
@@ -4903,6 +4879,29 @@ static const struct ethtool_ops ops = {
 	.self_test = nv_self_test,
 };
 
+static void nv_vlan_rx_register(struct net_device *dev, struct vlan_group *grp)
+{
+	struct fe_priv *np = get_nvpriv(dev);
+
+	spin_lock_irq(&np->lock);
+
+	/* save vlan group */
+	np->vlangrp = grp;
+
+	if (grp) {
+		/* enable vlan on MAC */
+		np->txrxctl_bits |= NVREG_TXRXCTL_VLANSTRIP | NVREG_TXRXCTL_VLANINS;
+	} else {
+		/* disable vlan on MAC */
+		np->txrxctl_bits &= ~NVREG_TXRXCTL_VLANSTRIP;
+		np->txrxctl_bits &= ~NVREG_TXRXCTL_VLANINS;
+	}
+
+	writel(np->txrxctl_bits, get_hwbase(dev) + NvRegTxRxControl);
+
+	spin_unlock_irq(&np->lock);
+}
+
 /* The mgmt unit and driver use a semaphore to access the phy during init */
 static int nv_mgmt_acquire_sema(struct net_device *dev)
 {
@@ -5209,6 +5208,7 @@ static const struct net_device_ops nv_netdev_ops = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address	= nv_set_mac_address,
 	.ndo_set_multicast_list	= nv_set_multicast,
+	.ndo_vlan_rx_register	= nv_vlan_rx_register,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= nv_poll_controller,
 #endif
@@ -5226,6 +5226,7 @@ static const struct net_device_ops nv_netdev_ops_optimized = {
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_set_mac_address	= nv_set_mac_address,
 	.ndo_set_multicast_list	= nv_set_multicast,
+	.ndo_vlan_rx_register	= nv_vlan_rx_register,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= nv_poll_controller,
 #endif
@@ -5338,16 +5339,15 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 		np->txrxctl_bits |= NVREG_TXRXCTL_RXCHECK;
 		dev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_SG |
 			NETIF_F_TSO | NETIF_F_RXCSUM;
+		dev->features |= dev->hw_features;
 	}
 
 	np->vlanctl_bits = 0;
 	if (id->driver_data & DEV_HAS_VLAN) {
 		np->vlanctl_bits = NVREG_VLANCONTROL_ENABLE;
-		dev->hw_features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX;
+		dev->features |= NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_TX;
 	}
 
-	dev->features |= dev->hw_features;
-
 	np->pause_flags = NV_PAUSEFRAME_RX_CAPABLE | NV_PAUSEFRAME_RX_REQ | NV_PAUSEFRAME_AUTONEG;
 	if ((id->driver_data & DEV_HAS_PAUSEFRAME_TX_V1) ||
 	    (id->driver_data & DEV_HAS_PAUSEFRAME_TX_V2) ||
@@ -5615,8 +5615,6 @@ static int __devinit nv_probe(struct pci_dev *pci_dev, const struct pci_device_i
 		goto out_error;
 	}
 
-	nv_vlan_mode(dev, dev->features);
-
 	netif_carrier_off(dev);
 
 	dev_info(&pci_dev->dev, "ifname %s, PHY OUI 0x%x @ %d, addr %pM\n",
diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 44da482..f2a4892 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -108,6 +108,12 @@ extern u16 vlan_dev_vlan_id(const struct net_device *dev);
 
 extern bool vlan_do_receive(struct sk_buff **skb);
 extern struct sk_buff *vlan_untag(struct sk_buff *skb);
+extern gro_result_t
+vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
+		 unsigned int vlan_tci, struct sk_buff *skb);
+extern gro_result_t
+vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
+	       unsigned int vlan_tci);
 
 #else
 static inline struct net_device *
@@ -139,6 +145,20 @@ static inline struct sk_buff *vlan_untag(struct sk_buff *skb)
 {
 	return skb;
 }
+
+static inline gro_result_t
+vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
+		 unsigned int vlan_tci, struct sk_buff *skb)
+{
+	return GRO_DROP;
+}
+
+static inline gro_result_t
+vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
+	       unsigned int vlan_tci)
+{
+	return GRO_DROP;
+}
 #endif
 
 /**
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index ddee79b..4537bff 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -768,6 +768,12 @@ struct netdev_tc_txq {
  *	3. Update dev->stats asynchronously and atomically, and define
  *	   neither operation.
  *
+ * void (*ndo_vlan_rx_register)(struct net_device *dev, struct vlan_group *grp);
+ *	If device support VLAN receive acceleration
+ *	(ie. dev->features & NETIF_F_HW_VLAN_RX), then this function is called
+ *	when vlan groups for the device changes.  Note: grp is NULL
+ *	if no vlan's groups are being used.
+ *
  * void (*ndo_vlan_rx_add_vid)(struct net_device *dev, unsigned short vid);
  *	If device support VLAN filtering (dev->features & NETIF_F_HW_VLAN_FILTER)
  *	this function is called when a VLAN id is registered.
@@ -886,6 +892,8 @@ struct net_device_ops {
 						     struct rtnl_link_stats64 *storage);
 	struct net_device_stats* (*ndo_get_stats)(struct net_device *dev);
 
+	void			(*ndo_vlan_rx_register)(struct net_device *dev,
+						        struct vlan_group *grp);
 	void			(*ndo_vlan_rx_add_vid)(struct net_device *dev,
 						       unsigned short vid);
 	void			(*ndo_vlan_rx_kill_vid)(struct net_device *dev,
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 8970ba1..d24c464 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -134,6 +134,8 @@ void unregister_vlan_dev(struct net_device *dev, struct list_head *head)
 		vlan_gvrp_uninit_applicant(real_dev);
 
 		rcu_assign_pointer(real_dev->vlgrp, NULL);
+		if (ops->ndo_vlan_rx_register)
+			ops->ndo_vlan_rx_register(real_dev, NULL);
 
 		/* Free the group, after all cpu's are done. */
 		call_rcu(&grp->rcu, vlan_rcu_free);
@@ -205,6 +207,8 @@ int register_vlan_dev(struct net_device *dev)
 	grp->nr_vlans++;
 
 	if (ngrp) {
+		if (ops->ndo_vlan_rx_register && (real_dev->features & NETIF_F_HW_VLAN_RX))
+			ops->ndo_vlan_rx_register(real_dev, ngrp);
 		rcu_assign_pointer(real_dev->vlgrp, ngrp);
 	}
 	if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
diff --git a/net/8021q/vlan_core.c b/net/8021q/vlan_core.c
index 5f27f8e..68b04ea 100644
--- a/net/8021q/vlan_core.c
+++ b/net/8021q/vlan_core.c
@@ -96,6 +96,22 @@ u16 vlan_dev_vlan_id(const struct net_device *dev)
 }
 EXPORT_SYMBOL(vlan_dev_vlan_id);
 
+gro_result_t vlan_gro_receive(struct napi_struct *napi, struct vlan_group *grp,
+			      unsigned int vlan_tci, struct sk_buff *skb)
+{
+	__vlan_hwaccel_put_tag(skb, vlan_tci);
+	return napi_gro_receive(napi, skb);
+}
+EXPORT_SYMBOL(vlan_gro_receive);
+
+gro_result_t vlan_gro_frags(struct napi_struct *napi, struct vlan_group *grp,
+			    unsigned int vlan_tci)
+{
+	__vlan_hwaccel_put_tag(napi->skb, vlan_tci);
+	return napi_gro_frags(napi);
+}
+EXPORT_SYMBOL(vlan_gro_frags);
+
 static struct sk_buff *vlan_reorder_header(struct sk_buff *skb)
 {
 	if (skb_cow(skb, skb_headroom(skb)) < 0)

^ permalink raw reply related

* RE: [PATCHv2 NEXT 1/1] netxen: add vlan accel support
From: Amit Salecha @ 2011-08-05 10:12 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Ameen Rahman, Rajesh Borundia
In-Reply-To: <20110805.024247.1468547132343070339.davem@davemloft.net>

> From: David Miller [mailto:davem@davemloft.net]
> Subject: Re: [PATCHv2 NEXT 1/1] netxen: add vlan accel support
> 
> From: <amit.salecha@qlogic.com>
> Date: Fri, 5 Aug 2011 02:20:15 -0700
> 
> > From: Rajesh K Borundia <rajesh.borundia@qlogic.com>
> >
> > o This increases performance on vlan interface.
> > o In case of fw reset, need to reprogram the ip addresses.
> > o Support LRO on vlan interface.
> >
> > Signed-off-by: Rajesh K Borundia <rajesh.borundia@qlogic.com>
> > Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> 
> As far as I can tell your card doesn't accelerate VLANs in hardware
> at all, is this true?
> 
True.

> I think it's very cumbersome and it doesn't make sense for every
> driver in your situation to add this same sequence of software calls.
> 
> Some generic facility and interfaces should be created so that the
> code doing this software VLAN acceleration is consolidated into
> one place and any driver can follow a simple to use API in order
> to make use of it.

Basically netxen_nic LRO requirement is to program ip addresses in HW.
After fw recovery we don't get vlan devices by any kernel api, so we are adding NETIF_VLAN_ACC support.
Earlier kernel had vlan group support, through that we get vlan devices and then their ip addresses.

So please suggest from below option:
   1) We can add kernel api to get vlan devices belongs to base interface
   2) we can maintain linked list of ip addresses belongs to vlan device in driver.
   3) Have same patch and write kernel api for vlan acceleration code.

-Amit


^ permalink raw reply

* Re: [PATCHv2 NEXT 1/1] netxen: add vlan accel support
From: David Miller @ 2011-08-05  9:42 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman, rajesh.borundia
In-Reply-To: <1312536015-21719-2-git-send-email-amit.salecha@qlogic.com>

From: <amit.salecha@qlogic.com>
Date: Fri, 5 Aug 2011 02:20:15 -0700

> From: Rajesh K Borundia <rajesh.borundia@qlogic.com>
> 
> o This increases performance on vlan interface.
> o In case of fw reset, need to reprogram the ip addresses.
> o Support LRO on vlan interface.
> 
> Signed-off-by: Rajesh K Borundia <rajesh.borundia@qlogic.com>
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>

As far as I can tell your card doesn't accelerate VLANs in hardware
at all, is this true?

I think it's very cumbersome and it doesn't make sense for every
driver in your situation to add this same sequence of software calls.

Some generic facility and interfaces should be created so that the
code doing this software VLAN acceleration is consolidated into
one place and any driver can follow a simple to use API in order
to make use of it.

^ permalink raw reply

* Hello My Dear, My name is Grace Adam,i read through your profile today and i became interested in you,i will also like to know you the more,and i want you to send an e-mail to my e-mail address so i can give you my picture for you to know whom i am and for the both of us to know each other very well and better in life,and we can achieve it in future because ture love and feeling means alot in future.Here is my private e-mail address you can contact me with it.(graceadam@gala.net)I am waiting for your mail to my e-mail address above and aslo, Remember the distance or colour does not matter but love,feeling,e-motions and sympathetic love matters alot in life. Thanks,and i promise to be honest and to keep a very good relationship with you. Grace.              graceadam@gala.net
From: graceadam @ 2011-08-05  9:33 UTC (permalink / raw)





^ permalink raw reply

* [PATCHv2 NEXT 1/1] netxen: add vlan accel support
From: amit.salecha @ 2011-08-05  9:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Rajesh K Borundia, Amit Kumar Salecha
In-Reply-To: <1312536015-21719-1-git-send-email-amit.salecha@qlogic.com>

From: Rajesh K Borundia <rajesh.borundia@qlogic.com>

o This increases performance on vlan interface.
o In case of fw reset, need to reprogram the ip addresses.
o Support LRO on vlan interface.

Signed-off-by: Rajesh K Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h      |    2 +
 drivers/net/netxen/netxen_nic_init.c |   22 ++++++++++++
 drivers/net/netxen/netxen_nic_main.c |   63 +++++++++++++++++++++++++++++----
 3 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index f744d29..b37cf4e 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -44,6 +44,7 @@
 #include <linux/timer.h>
 
 #include <linux/vmalloc.h>
+#include <linux/if_vlan.h>
 
 #include <asm/io.h>
 #include <asm/byteorder.h>
@@ -1212,6 +1213,7 @@ struct netxen_adapter {
 
 	u8 mac_addr[ETH_ALEN];
 
+	unsigned long vlans[BITS_TO_LONGS(VLAN_N_VID)];
 	struct netxen_adapter_stats stats;
 
 	struct netxen_recv_context recv_ctx;
diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index e8993a7..132d8fe 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -25,6 +25,7 @@
 
 #include <linux/netdevice.h>
 #include <linux/delay.h>
+#include <linux/if_vlan.h>
 #include <linux/slab.h>
 #include "netxen_nic.h"
 #include "netxen_nic_hw.h"
@@ -1558,7 +1559,9 @@ netxen_process_rcv(struct netxen_adapter *adapter,
 	struct netxen_rx_buffer *buffer;
 	struct sk_buff *skb;
 	struct nx_host_rds_ring *rds_ring;
+	struct ethhdr *eth_hdr;
 	int index, length, cksum, pkt_offset;
+	u16 vid = 0xffff;
 
 	if (unlikely(ring >= adapter->max_rds_rings))
 		return NULL;
@@ -1588,8 +1591,16 @@ netxen_process_rcv(struct netxen_adapter *adapter,
 	if (pkt_offset)
 		skb_pull(skb, pkt_offset);
 
+	if (!__vlan_get_tag(skb, &vid)) {
+		eth_hdr = (struct ethhdr *) skb->data;
+		memmove(skb->data + VLAN_HLEN, eth_hdr, ETH_ALEN * 2);
+		skb_pull(skb, VLAN_HLEN);
+	}
+
 	skb->protocol = eth_type_trans(skb, netdev);
 
+	if (vid != 0xffff)
+		__vlan_hwaccel_put_tag(skb, vid);
 	napi_gro_receive(&sds_ring->napi, skb);
 
 	adapter->stats.rx_pkts++;
@@ -1614,10 +1625,12 @@ netxen_process_lro(struct netxen_adapter *adapter,
 	struct nx_host_rds_ring *rds_ring;
 	struct iphdr *iph;
 	struct tcphdr *th;
+	struct ethhdr *eth_hdr;
 	bool push, timestamp;
 	int l2_hdr_offset, l4_hdr_offset;
 	int index;
 	u16 lro_length, length, data_offset;
+	u16 vid = 0xffff;
 	u32 seq_number;
 
 	if (unlikely(ring > adapter->max_rds_rings))
@@ -1650,6 +1663,13 @@ netxen_process_lro(struct netxen_adapter *adapter,
 	skb_put(skb, lro_length + data_offset);
 
 	skb_pull(skb, l2_hdr_offset);
+
+	if (!__vlan_get_tag(skb, &vid)) {
+		eth_hdr = (struct ethhdr *) skb->data;
+		memmove(skb->data + VLAN_HLEN, eth_hdr, ETH_ALEN * 2);
+		skb_pull(skb, VLAN_HLEN);
+	}
+
 	skb->protocol = eth_type_trans(skb, netdev);
 
 	iph = (struct iphdr *)skb->data;
@@ -1664,6 +1684,8 @@ netxen_process_lro(struct netxen_adapter *adapter,
 
 	length = skb->len;
 
+	if (vid != 0xffff)
+		__vlan_hwaccel_put_tag(skb, vid);
 	netif_receive_skb(skb);
 
 	adapter->stats.lro_pkts++;
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index f574edf..a6d6860 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -91,7 +91,11 @@ static irqreturn_t netxen_intr(int irq, void *data);
 static irqreturn_t netxen_msi_intr(int irq, void *data);
 static irqreturn_t netxen_msix_intr(int irq, void *data);
 
-static void netxen_config_indev_addr(struct net_device *dev, unsigned long);
+static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
+#ifdef CONFIG_INET
+static void netxen_config_indev_addr(struct netxen_adapter *,
+				struct net_device *, unsigned long);
+#endif
 static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
 						      struct rtnl_link_stats64 *stats);
 static int netxen_nic_set_mac(struct net_device *netdev, void *p);
@@ -517,6 +521,25 @@ static int netxen_set_features(struct net_device *dev, u32 features)
 	return 0;
 }
 
+static void
+netxen_vlan_rx_add(struct net_device *netdev, u16 vid)
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	set_bit(vid, adapter->vlans);
+}
+
+static void
+netxen_vlan_rx_del(struct net_device *netdev, u16 vid)
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	struct net_device *dev;
+
+	dev = __vlan_find_dev_deep(netdev, vid);
+	if (dev)
+		netxen_config_indev_addr(adapter, dev, NETDEV_DOWN);
+	clear_bit(vid, adapter->vlans);
+}
+
 static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_open	   = netxen_nic_open,
 	.ndo_stop	   = netxen_nic_close,
@@ -529,6 +552,8 @@ static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_tx_timeout	   = netxen_tx_timeout,
 	.ndo_fix_features = netxen_fix_features,
 	.ndo_set_features = netxen_set_features,
+	.ndo_vlan_rx_add_vid	= netxen_vlan_rx_add,
+	.ndo_vlan_rx_kill_vid	= netxen_vlan_rx_del,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = netxen_nic_poll_controller,
 #endif
@@ -1259,6 +1284,9 @@ netxen_setup_netdev(struct netxen_adapter *adapter,
 	if (adapter->capabilities & NX_FW_CAPABILITY_FVLANTX)
 		netdev->hw_features |= NETIF_F_HW_VLAN_TX;
 
+	netdev->hw_features |= NETIF_F_HW_VLAN_RX;
+	netdev->hw_features |= NETIF_F_HW_VLAN_FILTER;
+
 	if (adapter->capabilities & NX_FW_CAPABILITY_HW_LRO)
 		netdev->hw_features |= NETIF_F_LRO;
 
@@ -1563,7 +1591,7 @@ static int netxen_nic_attach_func(struct pci_dev *pdev)
 		if (err)
 			goto err_out_detach;
 
-		netxen_config_indev_addr(netdev, NETDEV_UP);
+		netxen_restore_indev_addr(netdev, NETDEV_UP);
 	}
 
 	netif_device_attach(netdev);
@@ -2374,7 +2402,7 @@ netxen_attach_work(struct work_struct *work)
 			goto done;
 		}
 
-		netxen_config_indev_addr(netdev, NETDEV_UP);
+		netxen_restore_indev_addr(netdev, NETDEV_UP);
 	}
 
 	netif_device_attach(netdev);
@@ -2848,10 +2876,10 @@ netxen_destip_supported(struct netxen_adapter *adapter)
 }
 
 static void
-netxen_config_indev_addr(struct net_device *dev, unsigned long event)
+netxen_config_indev_addr(struct netxen_adapter *adapter,
+		struct net_device *dev, unsigned long event)
 {
 	struct in_device *indev;
-	struct netxen_adapter *adapter = netdev_priv(dev);
 
 	if (!netxen_destip_supported(adapter))
 		return;
@@ -2878,6 +2906,24 @@ netxen_config_indev_addr(struct net_device *dev, unsigned long event)
 	in_dev_put(indev);
 }
 
+static void
+netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
+
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	struct net_device *dev;
+	u16 vid;
+
+	netxen_config_indev_addr(adapter, netdev, event);
+
+	for_each_set_bit(vid, adapter->vlans, VLAN_N_VID) {
+		dev = __vlan_find_dev_deep(netdev, vid);
+		if (!dev)
+			continue;
+		netxen_config_indev_addr(adapter, dev, event);
+	}
+}
+
 static int netxen_netdev_event(struct notifier_block *this,
 				 unsigned long event, void *ptr)
 {
@@ -2904,7 +2950,7 @@ recheck:
 	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
 		goto done;
 
-	netxen_config_indev_addr(dev, event);
+	netxen_config_indev_addr(adapter, dev, event);
 done:
 	return NOTIFY_DONE;
 }
@@ -2921,7 +2967,7 @@ netxen_inetaddr_event(struct notifier_block *this,
 	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
 
 recheck:
-	if (dev == NULL || !netif_running(dev))
+	if (dev == NULL)
 		goto done;
 
 	if (dev->priv_flags & IFF_802_1Q_VLAN) {
@@ -2964,8 +3010,9 @@ static struct notifier_block netxen_inetaddr_cb = {
 };
 #else
 static void
-netxen_config_indev_addr(struct net_device *dev, unsigned long event)
+netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
 { }
+static void
 #endif
 
 static struct pci_error_handlers netxen_err_handler = {
-- 
1.7.3.3



^ permalink raw reply related

* [PATCHv2 NEXT 0/1] netxen: update
From: amit.salecha @ 2011-08-05  9:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman

Hi
	Resending patch with correct author email address.
	Apply this to net-next branch.
-Amit


^ permalink raw reply

* [PATCH NEXT 1/1] netxen: add vlan accel support
From: amit.salecha @ 2011-08-05  8:59 UTC (permalink / raw)
  To: davem
  Cc: netdev, ameen.rahman, Rajesh K Borundia, Rajesh K Borundia,
	Amit Kumar Salecha

From: Rajesh K Borundia <rajeshk@dut4145.unminc.com>

o This increases performance on vlan interface.
o In case of fw reset, need to reprogram the ip addresses.
o Support LRO on vlan interface.

Signed-off-by: Rajesh K Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h      |    2 +
 drivers/net/netxen/netxen_nic_init.c |   22 ++++++++++++
 drivers/net/netxen/netxen_nic_main.c |   63 +++++++++++++++++++++++++++++----
 3 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index f744d29..b37cf4e 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -44,6 +44,7 @@
 #include <linux/timer.h>
 
 #include <linux/vmalloc.h>
+#include <linux/if_vlan.h>
 
 #include <asm/io.h>
 #include <asm/byteorder.h>
@@ -1212,6 +1213,7 @@ struct netxen_adapter {
 
 	u8 mac_addr[ETH_ALEN];
 
+	unsigned long vlans[BITS_TO_LONGS(VLAN_N_VID)];
 	struct netxen_adapter_stats stats;
 
 	struct netxen_recv_context recv_ctx;
diff --git a/drivers/net/netxen/netxen_nic_init.c b/drivers/net/netxen/netxen_nic_init.c
index e8993a7..132d8fe 100644
--- a/drivers/net/netxen/netxen_nic_init.c
+++ b/drivers/net/netxen/netxen_nic_init.c
@@ -25,6 +25,7 @@
 
 #include <linux/netdevice.h>
 #include <linux/delay.h>
+#include <linux/if_vlan.h>
 #include <linux/slab.h>
 #include "netxen_nic.h"
 #include "netxen_nic_hw.h"
@@ -1558,7 +1559,9 @@ netxen_process_rcv(struct netxen_adapter *adapter,
 	struct netxen_rx_buffer *buffer;
 	struct sk_buff *skb;
 	struct nx_host_rds_ring *rds_ring;
+	struct ethhdr *eth_hdr;
 	int index, length, cksum, pkt_offset;
+	u16 vid = 0xffff;
 
 	if (unlikely(ring >= adapter->max_rds_rings))
 		return NULL;
@@ -1588,8 +1591,16 @@ netxen_process_rcv(struct netxen_adapter *adapter,
 	if (pkt_offset)
 		skb_pull(skb, pkt_offset);
 
+	if (!__vlan_get_tag(skb, &vid)) {
+		eth_hdr = (struct ethhdr *) skb->data;
+		memmove(skb->data + VLAN_HLEN, eth_hdr, ETH_ALEN * 2);
+		skb_pull(skb, VLAN_HLEN);
+	}
+
 	skb->protocol = eth_type_trans(skb, netdev);
 
+	if (vid != 0xffff)
+		__vlan_hwaccel_put_tag(skb, vid);
 	napi_gro_receive(&sds_ring->napi, skb);
 
 	adapter->stats.rx_pkts++;
@@ -1614,10 +1625,12 @@ netxen_process_lro(struct netxen_adapter *adapter,
 	struct nx_host_rds_ring *rds_ring;
 	struct iphdr *iph;
 	struct tcphdr *th;
+	struct ethhdr *eth_hdr;
 	bool push, timestamp;
 	int l2_hdr_offset, l4_hdr_offset;
 	int index;
 	u16 lro_length, length, data_offset;
+	u16 vid = 0xffff;
 	u32 seq_number;
 
 	if (unlikely(ring > adapter->max_rds_rings))
@@ -1650,6 +1663,13 @@ netxen_process_lro(struct netxen_adapter *adapter,
 	skb_put(skb, lro_length + data_offset);
 
 	skb_pull(skb, l2_hdr_offset);
+
+	if (!__vlan_get_tag(skb, &vid)) {
+		eth_hdr = (struct ethhdr *) skb->data;
+		memmove(skb->data + VLAN_HLEN, eth_hdr, ETH_ALEN * 2);
+		skb_pull(skb, VLAN_HLEN);
+	}
+
 	skb->protocol = eth_type_trans(skb, netdev);
 
 	iph = (struct iphdr *)skb->data;
@@ -1664,6 +1684,8 @@ netxen_process_lro(struct netxen_adapter *adapter,
 
 	length = skb->len;
 
+	if (vid != 0xffff)
+		__vlan_hwaccel_put_tag(skb, vid);
 	netif_receive_skb(skb);
 
 	adapter->stats.lro_pkts++;
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index f574edf..a6d6860 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -91,7 +91,11 @@ static irqreturn_t netxen_intr(int irq, void *data);
 static irqreturn_t netxen_msi_intr(int irq, void *data);
 static irqreturn_t netxen_msix_intr(int irq, void *data);
 
-static void netxen_config_indev_addr(struct net_device *dev, unsigned long);
+static void netxen_restore_indev_addr(struct net_device *dev, unsigned long);
+#ifdef CONFIG_INET
+static void netxen_config_indev_addr(struct netxen_adapter *,
+				struct net_device *, unsigned long);
+#endif
 static struct rtnl_link_stats64 *netxen_nic_get_stats(struct net_device *dev,
 						      struct rtnl_link_stats64 *stats);
 static int netxen_nic_set_mac(struct net_device *netdev, void *p);
@@ -517,6 +521,25 @@ static int netxen_set_features(struct net_device *dev, u32 features)
 	return 0;
 }
 
+static void
+netxen_vlan_rx_add(struct net_device *netdev, u16 vid)
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	set_bit(vid, adapter->vlans);
+}
+
+static void
+netxen_vlan_rx_del(struct net_device *netdev, u16 vid)
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	struct net_device *dev;
+
+	dev = __vlan_find_dev_deep(netdev, vid);
+	if (dev)
+		netxen_config_indev_addr(adapter, dev, NETDEV_DOWN);
+	clear_bit(vid, adapter->vlans);
+}
+
 static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_open	   = netxen_nic_open,
 	.ndo_stop	   = netxen_nic_close,
@@ -529,6 +552,8 @@ static const struct net_device_ops netxen_netdev_ops = {
 	.ndo_tx_timeout	   = netxen_tx_timeout,
 	.ndo_fix_features = netxen_fix_features,
 	.ndo_set_features = netxen_set_features,
+	.ndo_vlan_rx_add_vid	= netxen_vlan_rx_add,
+	.ndo_vlan_rx_kill_vid	= netxen_vlan_rx_del,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = netxen_nic_poll_controller,
 #endif
@@ -1259,6 +1284,9 @@ netxen_setup_netdev(struct netxen_adapter *adapter,
 	if (adapter->capabilities & NX_FW_CAPABILITY_FVLANTX)
 		netdev->hw_features |= NETIF_F_HW_VLAN_TX;
 
+	netdev->hw_features |= NETIF_F_HW_VLAN_RX;
+	netdev->hw_features |= NETIF_F_HW_VLAN_FILTER;
+
 	if (adapter->capabilities & NX_FW_CAPABILITY_HW_LRO)
 		netdev->hw_features |= NETIF_F_LRO;
 
@@ -1563,7 +1591,7 @@ static int netxen_nic_attach_func(struct pci_dev *pdev)
 		if (err)
 			goto err_out_detach;
 
-		netxen_config_indev_addr(netdev, NETDEV_UP);
+		netxen_restore_indev_addr(netdev, NETDEV_UP);
 	}
 
 	netif_device_attach(netdev);
@@ -2374,7 +2402,7 @@ netxen_attach_work(struct work_struct *work)
 			goto done;
 		}
 
-		netxen_config_indev_addr(netdev, NETDEV_UP);
+		netxen_restore_indev_addr(netdev, NETDEV_UP);
 	}
 
 	netif_device_attach(netdev);
@@ -2848,10 +2876,10 @@ netxen_destip_supported(struct netxen_adapter *adapter)
 }
 
 static void
-netxen_config_indev_addr(struct net_device *dev, unsigned long event)
+netxen_config_indev_addr(struct netxen_adapter *adapter,
+		struct net_device *dev, unsigned long event)
 {
 	struct in_device *indev;
-	struct netxen_adapter *adapter = netdev_priv(dev);
 
 	if (!netxen_destip_supported(adapter))
 		return;
@@ -2878,6 +2906,24 @@ netxen_config_indev_addr(struct net_device *dev, unsigned long event)
 	in_dev_put(indev);
 }
 
+static void
+netxen_restore_indev_addr(struct net_device *netdev, unsigned long event)
+
+{
+	struct netxen_adapter *adapter = netdev_priv(netdev);
+	struct net_device *dev;
+	u16 vid;
+
+	netxen_config_indev_addr(adapter, netdev, event);
+
+	for_each_set_bit(vid, adapter->vlans, VLAN_N_VID) {
+		dev = __vlan_find_dev_deep(netdev, vid);
+		if (!dev)
+			continue;
+		netxen_config_indev_addr(adapter, dev, event);
+	}
+}
+
 static int netxen_netdev_event(struct notifier_block *this,
 				 unsigned long event, void *ptr)
 {
@@ -2904,7 +2950,7 @@ recheck:
 	if (adapter->is_up != NETXEN_ADAPTER_UP_MAGIC)
 		goto done;
 
-	netxen_config_indev_addr(dev, event);
+	netxen_config_indev_addr(adapter, dev, event);
 done:
 	return NOTIFY_DONE;
 }
@@ -2921,7 +2967,7 @@ netxen_inetaddr_event(struct notifier_block *this,
 	dev = ifa->ifa_dev ? ifa->ifa_dev->dev : NULL;
 
 recheck:
-	if (dev == NULL || !netif_running(dev))
+	if (dev == NULL)
 		goto done;
 
 	if (dev->priv_flags & IFF_802_1Q_VLAN) {
@@ -2964,8 +3010,9 @@ static struct notifier_block netxen_inetaddr_cb = {
 };
 #else
 static void
-netxen_config_indev_addr(struct net_device *dev, unsigned long event)
+netxen_restore_indev_addr(struct net_device *dev, unsigned long event)
 { }
+static void
 #endif
 
 static struct pci_error_handlers netxen_err_handler = {
-- 
1.7.3.3



^ permalink raw reply related

* [PATCH 5/5] Export the tcp sock's security context to proc.
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux
In-Reply-To: <1312534686-4099-1-git-send-email-rongqing.li@windriver.com>

From: Roy.Li <rongqing.li@windriver.com>

Export the tcp sock's security context to proc, since it maybe
different from the sock's owner process security context.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 net/ipv4/tcp_ipv4.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 955b8e6..98a85d6 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2478,13 +2478,16 @@ static void get_timewait4_sock(struct inet_timewait_sock *tw,
 static int tcp4_seq_show(struct seq_file *seq, void *v)
 {
 	struct tcp_iter_state *st;
-	int len;
+	int len, sclen;
+	struct sock *s = NULL;
 
 	if (v == SEQ_START_TOKEN) {
-		seq_printf(seq, "%-*s\n", TMPSZ - 1,
+		seq_printf(seq,
 			   "  sl  local_address rem_address   st tx_queue "
 			   "rx_queue tr tm->when retrnsmt   uid  timeout "
-			   "inode");
+			   "inode %n", &len);
+		seq_printf(seq, "%-*s\n", TMPSZ - len - 1,
+			   (selinux_is_enabled() ? "  scontext" : ""));
 		goto out;
 	}
 	st = seq->private;
@@ -2493,15 +2496,19 @@ static int tcp4_seq_show(struct seq_file *seq, void *v)
 	case TCP_SEQ_STATE_LISTENING:
 	case TCP_SEQ_STATE_ESTABLISHED:
 		get_tcp4_sock(v, seq, st->num, &len);
+		s = v;
 		break;
 	case TCP_SEQ_STATE_OPENREQ:
 		get_openreq4(st->syn_wait_sk, v, seq, st->num, st->uid, &len);
+		s = st->syn_wait_sk;
 		break;
 	case TCP_SEQ_STATE_TIME_WAIT:
 		get_timewait4_sock(v, seq, st->num, &len);
 		break;
 	}
-	seq_printf(seq, "%*s\n", TMPSZ - 1 - len, "");
+
+	sock_write_secctx(s, seq, &sclen);
+	seq_printf(seq, "%*s\n", TMPSZ + sclen - 1 - len, "");
 out:
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related

* [PATCH 4/5] Export the unix sock's security context to proc.
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux
In-Reply-To: <1312534686-4099-1-git-send-email-rongqing.li@windriver.com>

From: Roy.Li <rongqing.li@windriver.com>

Export the unix sock's security context to proc, since it maybe
different from the sock's owner process security context.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 net/unix/af_unix.c |   15 +++++++++++++--
 1 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index ec68e1c..338fa0a 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2239,12 +2239,15 @@ static void unix_seq_stop(struct seq_file *seq, void *v)
 	spin_unlock(&unix_table_lock);
 }
 
+#define OFFSET_PATH_START	40
 static int unix_seq_show(struct seq_file *seq, void *v)
 {
+	int seqlen, offset = OFFSET_PATH_START;
 
 	if (v == SEQ_START_TOKEN)
-		seq_puts(seq, "Num       RefCount Protocol Flags    Type St "
-			 "Inode Path\n");
+		seq_printf(seq, "Num       RefCount Protocol Flags    Type St "
+			 "Inode Path    %s",
+			 (selinux_is_enabled() ? "  scontext\n" : "\n"));
 	else {
 		struct sock *s = v;
 		struct unix_sock *u = unix_sk(s);
@@ -2275,7 +2278,15 @@ static int unix_seq_show(struct seq_file *seq, void *v)
 			}
 			for ( ; i < len; i++)
 				seq_putc(seq, u->addr->name->sun_path[i]);
+			if (offset > len)
+				offset -= (len + 1);
+			else
+				offset = 0;
 		}
+
+		seq_printf(seq, "%*c", offset, '\b');
+		sock_write_secctx(s, seq, &seqlen);
+
 		unix_state_unlock(s);
 		seq_putc(seq, '\n');
 	}
-- 
1.7.1


^ permalink raw reply related

* [PATCH 3/5] Export the udp sock's security context to proc.
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux
In-Reply-To: <1312534686-4099-1-git-send-email-rongqing.li@windriver.com>

From: Roy.Li <rongqing.li@windriver.com>

Export the udp sock's security context to proc, since it maybe
different from the sock's owner process security context.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 net/ipv4/udp.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 1b5a193..e64b858 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -2089,6 +2089,7 @@ static void udp4_format_sock(struct sock *sp, struct seq_file *f,
 		int bucket, int *len)
 {
 	struct inet_sock *inet = inet_sk(sp);
+	int sclen;
 	__be32 dest = inet->inet_daddr;
 	__be32 src  = inet->inet_rcv_saddr;
 	__u16 destp	  = ntohs(inet->inet_dport);
@@ -2102,21 +2103,29 @@ static void udp4_format_sock(struct sock *sp, struct seq_file *f,
 		0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
 		atomic_read(&sp->sk_refcnt), sp,
 		atomic_read(&sp->sk_drops), len);
+
+	sock_write_secctx(sp, f, &sclen);
+	*len += sclen;
 }
 
 int udp4_seq_show(struct seq_file *seq, void *v)
 {
-	if (v == SEQ_START_TOKEN)
-		seq_printf(seq, "%-127s\n",
+	int len;
+
+	if (v == SEQ_START_TOKEN) {
+		seq_printf(seq,
 			   "  sl  local_address rem_address   st tx_queue "
 			   "rx_queue tr tm->when retrnsmt   uid  timeout "
-			   "inode ref pointer drops");
+			   "inode ref pointer drops%n",
+			   &len);
+		seq_printf(seq, "%-*s\n", 150 - len,
+			   (selinux_is_enabled() ? "  scontext" : ""));
+	}
 	else {
 		struct udp_iter_state *state = seq->private;
-		int len;
 
 		udp4_format_sock(v, seq, state->bucket, &len);
-		seq_printf(seq, "%*s\n", 127 - len, "");
+		seq_printf(seq, "%*s\n", 150 - len, "");
 	}
 	return 0;
 }
-- 
1.7.1


^ permalink raw reply related

* [PATCH 2/5] Export the raw sock's security context to proc.
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux
In-Reply-To: <1312534686-4099-1-git-send-email-rongqing.li@windriver.com>

From: Roy.Li <rongqing.li@windriver.com>

The element sk_security of struct sock represents the socket
security context ID, which is inheriting from the process when
creates this socket on most of the time.

but when SELinux type_transition rule is applied to socket, or
application sets /proc/xxx/attr/createsock, the socket security
context would be different from the creating process. on this
condition, the "netstat -Z" will return wrong value, since
"netstat -Z" only returns the process security context as socket
process security.

Export the raw sock's security context to proc, so that "netstat -Z"
could be fixed by reading procfs.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 net/ipv4/raw.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 1457acb..645d373 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -972,6 +972,7 @@ EXPORT_SYMBOL_GPL(raw_seq_stop);
 
 static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)
 {
+	int sclen;
 	struct inet_sock *inet = inet_sk(sp);
 	__be32 dest = inet->inet_daddr,
 	       src = inet->inet_rcv_saddr;
@@ -979,12 +980,15 @@ static void raw_sock_seq_show(struct seq_file *seq, struct sock *sp, int i)
 	      srcp  = inet->inet_num;
 
 	seq_printf(seq, "%4d: %08X:%04X %08X:%04X"
-		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %pK %d\n",
+		" %02X %08X:%08X %02X:%08lX %08X %5d %8d %lu %d %pK %d",
 		i, src, srcp, dest, destp, sp->sk_state,
 		sk_wmem_alloc_get(sp),
 		sk_rmem_alloc_get(sp),
 		0, 0L, 0, sock_i_uid(sp), 0, sock_i_ino(sp),
 		atomic_read(&sp->sk_refcnt), sp, atomic_read(&sp->sk_drops));
+
+	sock_write_secctx(sp, seq, &sclen);
+	seq_putc(seq, '\n');
 }
 
 static int raw_seq_show(struct seq_file *seq, void *v)
@@ -992,7 +996,8 @@ static int raw_seq_show(struct seq_file *seq, void *v)
 	if (v == SEQ_START_TOKEN)
 		seq_printf(seq, "  sl  local_address rem_address   st tx_queue "
 				"rx_queue tr tm->when retrnsmt   uid  timeout "
-				"inode ref pointer drops\n");
+				"inode ref pointer drops %s",
+				(selinux_is_enabled() ? "  scontext\n" : "\n"));
 	else
 		raw_sock_seq_show(seq, v, raw_seq_private(seq)->bucket);
 	return 0;
-- 
1.7.1


^ permalink raw reply related

* [PATCH 1/5] Define the function to write sock's security context to seq_file.
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux
In-Reply-To: <1312534686-4099-1-git-send-email-rongqing.li@windriver.com>

From: Roy.Li <rongqing.li@windriver.com>

This function will write the sock's security context to a seq_file
and return the error code, and the number of characters successfully
written is written in int pointers parameter.

This function will be called when export socket information to proc.

Signed-off-by: Roy.Li <rongqing.li@windriver.com>
---
 include/net/sock.h |    1 +
 net/core/sock.c    |   26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 8e4062f..0366ab1 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1828,6 +1828,7 @@ static inline struct sock *skb_steal_sock(struct sk_buff *skb)
 extern void sock_enable_timestamp(struct sock *sk, int flag);
 extern int sock_get_timestamp(struct sock *, struct timeval __user *);
 extern int sock_get_timestampns(struct sock *, struct timespec __user *);
+extern int sock_write_secctx(struct sock *sk, struct seq_file *seq, int *len);
 
 /* 
  *	Enable debug/info messages 
diff --git a/net/core/sock.c b/net/core/sock.c
index bc745d0..1126a49 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2254,6 +2254,32 @@ void sk_common_release(struct sock *sk)
 }
 EXPORT_SYMBOL(sk_common_release);
 
+int sock_write_secctx(struct sock *sk, struct seq_file *seq, int *len)
+{
+	struct flowi fl;
+	char *ctx = NULL;
+	u32 ctxlen;
+	int res = 0;
+
+	*len = 0;
+
+	if (sk == NULL)
+		return -EINVAL;
+	res = security_socket_getsockname(sk->sk_socket);
+	if (res)
+		return res;
+
+	security_sk_classify_flow(sk, &fl);
+
+	res = security_secid_to_secctx(fl.flowi_secid, &ctx, &ctxlen);
+	if (res)
+		return res;
+
+	seq_printf(seq, " %s%n", ctx, len);
+	security_release_secctx(ctx, ctxlen);
+	return res;
+}
+
 static DEFINE_RWLOCK(proto_list_lock);
 static LIST_HEAD(proto_list);
 
-- 
1.7.1


^ permalink raw reply related

* [PATCH 0/5] Export the sock's security context to proc
From: rongqing.li @ 2011-08-05  8:58 UTC (permalink / raw)
  To: netdev, selinux

-------
    Any review would be much appreciated.
 
Comments:
--------
    Export the sock's security context to proc.
    
    The element sk_security of struct sock represents the socket
    security context ID, which is inheriting from the process when
    creates this socket on most of the time.
    
    but when SELinux type_transition rule is applied to socket, or
    application sets /proc/xxx/attr/createsock, the socket security
    context would be different from the creating process. on this
    condition, the "netstat -Z" will return wrong value, since
    "netstat -Z" only returns the process security context as socket
    process security.
    
    Export the raw sock's security context to proc, so that "netstat -Z"
    could be fixed by reading procfs.

Test:
--------
1. When Enable SELinux.


1.1 check the socket security context has been exported in procfs

root@qemu-host:/root> head -n 3 /proc/net/tcp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode   scontext                                          
   0: 00000000:05FE 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 5029 1 ffff88001b8ecc00 100 0 0 10 -1 system_u:system_r:initrc_t:s0-s15:c0.c1023                                                                
   1: 00000000:DBE2 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 4915 1 ffff88001b8ec600 100 0 0 10 -1 system_u:system_r:rpcd_t:s0-s15:c0.c1023                                                              


root@qemu-host:/root> head -n 3 /proc/net/udp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops  scontext                          
   53: 00000000:89F1 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 4912 2 ffff88001e3b49c0 0 system_u:system_r:rpcd_t:s0-s15:c0.c1023        
  172: 00000000:0268 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 4851 2 ffff88001e3b4340 0 system_u:system_r:rpcbind_t:s0-s15:c0.c1023           


root@qemu-host:/root> head -n 3 /proc/net/unix 
Num       RefCount Protocol Flags    Type St Inode Path      scontext
ffff88001ea1cc00: 00000002 00000000 00000000 0002 01   976 @/org/kernel/udev/udevd               system_u:system_r:udev_t:s0-s15:c0.c1023
ffff88001bbe6600: 0000000A 00000000 00000000 0002 01  4740 /dev/log                              system_u:system_r:syslogd_s_t:s15:c0.c1023
root@qemu-host:/root> 


root@qemu-host:/root> head -n 3 /proc/net/raw  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops   scontext
root@qemu-host:/root> 

1.2 check these patches do not affect the netstat, it can still work
root@qemu-host:/root> netstat -a
Active Internet connections (servers and established) Proto Recv-Q Send-Q Local Address               Foreign Address State      
tcp        0      0 *:1534                      *:* LISTEN      
tcp        0      0 *:56290                     *:* LISTEN      
tcp        0      0 localhost:submission        *:* LISTEN      
tcp        0      0 *:sunrpc                    *:* LISTEN
...

1.3 When syslog creates socket, and type transition has been applied on them, the security context of
socket would be syslogd_s_t, not same as its own process security context
syslogd_t, the "netstat -Z" returns wrong value, but the security context in procfs is correct

root@qemu-host:/etc> cat /proc/net/unix |grep syslog
ffff88001f856000: 00000002 00000000 00010000 0001 01  6385 /var/lib/syslog-ng/syslog-ng.ctl      system_u:system_r:syslogd_t:s15:c0.c1023
ffff88001f856300: 00000002 00000000 00000000 0002 01  6383 /dev/log                              system_u:system_r:syslogd_s_t:s15:c0.c1023
root@qemu-host:/etc> 

root@qemu-host:/etc> netstat -aZ|grep 6383
unix  2      [ ]         DGRAM                    6383   793/syslog-ng
system_u:system_r:syslogd_t:s15:c0.c1023          /dev/log
root@qemu-host:/etc> 



2. When SElinux is disabled

2.1 check the /proc/net/udp information are same as no these patches

root@qemu-host:/root> head -n 3 /proc/net/raw  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops 

root@qemu-host:/root> head -n 3 /proc/net/unix 
Num       RefCount Protocol Flags    Type St Inode Path    
ffff88001d226000: 0000000A 00000000 00000000 0002 01  2661 /dev/log                              
ffff88001ea1cc00: 00000002 00000000 00000000 0002 01   897 @/org/kernel/udev/udevd               

root@qemu-host:/root> head -n 3 /proc/net/tcp  
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode                                                     
   0: 00000000:05FE 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 2950 1 ffff88001d294c00 100 0 0 10 -1                     
   1: 0100007F:024B 00000000:0000 0A 00000000:00000000 00:00000000 00000000 0        0 3217 1 ffff88001d295e00 100 0 0 10 -1                     

root@qemu-host:/root> head -n 3 /proc/net/udp 
  sl  local_address rem_address   st tx_queue rx_queue tr tm->when retrnsmt uid  timeout inode ref pointer drops                                    
   57: 00000000:03F5 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 2772 2 ffff88001d2ac340 0                                 
  122: 00000000:D936 00000000:0000 07 00000000:00000000 00:00000000 00000000 0        0 2831 2 ffff88001d2acd00 0                                 
root@qemu-host:/root>



^ permalink raw reply

* Re: [RFC 2/4] [flexcan] Introduce a flexcan_clk set of functions.
From: Marc Kleine-Budde @ 2011-08-05  8:34 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-3-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 6442 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> The freescale P1010RDB board does not have a
> clk_{get,put,get_rate,enable,disable} set of functions.  Wrap these with a
> flexcan_ #define for arm, and implement a more complete function for ppc.

Some codingstyle nitpicks inline. I hope we'll find a cleaner solution
than this patch.

Marc

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |  114 +++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 105 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 74b1706..3417d0b 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -220,6 +220,102 @@ static inline void flexcan_write(u32 val, void __iomem *addr)
>  }
>  #endif
>  
> +#if defined(__powerpc__)
> +struct flexcan_clk {
> +	unsigned long rate;
> +	void *data;
> +};
> +
> +static struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	struct flexcan_clk *clock;
> +	u32 *clock_freq;
> +	u32 *clock_divider;
> +	int err;
> +
> +	clock = kmalloc(sizeof(struct flexcan_clk), GFP_KERNEL);
> +	if (!clock) {
> +		dev_err(dev, "Cannot allocate memory\n");
> +		err = -ENOMEM;
> +		goto failed_clock;
> +	}
> +	clock_freq = (u32 *)of_get_property(dev->of_node, "clock_freq", NULL);
> +	if (!clock_freq) {
> +		dev_err(dev, "Cannot find clock_freq property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock_divider = (u32 *) of_get_property(dev->of_node,
                               ^

remove this space, please
> +					"fsl,flexcan-clock-divider", NULL);
> +	if (clock_divider == NULL) {

!clock_divider

> +		dev_err(dev, "Cannot find fsl,flexcan-clock-divider property\n");
> +		err = -EINVAL;
> +		goto failed_clock;
> +	}
> +
> +	clock->rate = DIV_ROUND_CLOSEST(*clock_freq / *clock_divider, 1000);
> +	clock->rate *= 1000;
> +
> +	return (struct clk *)clock;
> +
> + failed_clock:
> +	kfree(clock);
> +	return ERR_PTR(err);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;

that cast is not needed.

> +
> +	kfree(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return 0;
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	return;
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *_clk)
> +{
> +	struct flexcan_clk *clk = (struct flexcan_clk *)_clk;
> +
> +	return clk->rate;
> +}
> +
> +#else
> +static inline struct clk *flexcan_clk_get(struct device *dev, const char *id)
> +{
> +	return clk_get(dev, id);
> +}
> +
> +static inline void flexcan_clk_put(struct clk *clk)
> +{
> +	clk_put(clk);
> +}
> +
> +static inline int flexcan_clk_enable(struct clk *clk)
> +{
> +	return clk_enable(clk);
> +}
> +
> +static inline void flexcan_clk_disable(struct clk *clk)
> +{
> +	clk_disable(clk);
> +}
> +
> +static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
> +{
> +	return clk_get_rate(clk);
> +}
> +
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -807,7 +903,7 @@ static int flexcan_open(struct net_device *dev)
>  	struct flexcan_priv *priv = netdev_priv(dev);
>  	int err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	err = open_candev(dev);
>  	if (err)
> @@ -829,7 +925,7 @@ static int flexcan_open(struct net_device *dev)
>   out_close:
>  	close_candev(dev);
>   out:
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -843,7 +939,7 @@ static int flexcan_close(struct net_device *dev)
>  	flexcan_chip_stop(dev);
>  
>  	free_irq(dev->irq, dev);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	close_candev(dev);
>  
> @@ -882,7 +978,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg, err;
>  
> -	clk_enable(priv->clk);
> +	flexcan_clk_enable(priv->clk);
>  
>  	/* select "bus clock", chip must be disabled */
>  	flexcan_chip_disable(priv);
> @@ -916,7 +1012,7 @@ static int __devinit register_flexcandev(struct net_device *dev)
>   out:
>  	/* disable core and turn off clocks */
>  	flexcan_chip_disable(priv);
> -	clk_disable(priv->clk);
> +	flexcan_clk_disable(priv->clk);
>  
>  	return err;
>  }
> @@ -936,7 +1032,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	resource_size_t mem_size;
>  	int err, irq;
>  
> -	clk = clk_get(&pdev->dev, NULL);
> +	clk = flexcan_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(clk)) {
>  		dev_err(&pdev->dev, "no clock defined\n");
>  		err = PTR_ERR(clk);
> @@ -973,7 +1069,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>  	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
>  
>  	priv = netdev_priv(dev);
> -	priv->can.clock.freq = clk_get_rate(clk);
> +	priv->can.clock.freq = flexcan_clk_get_rate(clk);
>  	priv->can.bittiming_const = &flexcan_bittiming_const;
>  	priv->can.do_set_mode = flexcan_set_mode;
>  	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
> @@ -1008,7 +1104,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
>   failed_map:
>  	release_mem_region(mem->start, mem_size);
>   failed_get:
> -	clk_put(clk);
> +	flexcan_clk_put(clk);
>   failed_clock:
>  	return err;
>  }
> @@ -1026,7 +1122,7 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));
>  
> -	clk_put(priv->clk);
> +	flexcan_clk_put(priv->clk);
>  
>  	free_candev(dev);
>  


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [RFC 1/4] [flexcan] Abstract off read/write for big/little endian.
From: Marc Kleine-Budde @ 2011-08-05  8:32 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-2-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 858 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> First step in converting the flexcan driver from supporting just arm to
> supporting both arm and powerpc architectures.
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Acked-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 1/3] net: sendmmsg should only return an error if no messages were sent
From: Steven Whitehouse @ 2011-08-05  8:20 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: acme, rdenis, netdev
In-Reply-To: <201108050357.p753vtpO022773@www262.sakura.ne.jp>

Hi,

On Fri, 2011-08-05 at 12:57 +0900, Tetsuo Handa wrote:
> Anton Blanchard wrote:
> > sendmmsg uses a similar error return strategy as recvmmsg but it
> > turns out to be a confusing way to communicate errors.
> > 
> > The current code stores the error code away and returns it on the next
> > sendmmsg call. This means a call with completely valid arguments could
> > get an error from a previous call.
> > 
> > Change things so we only return an error if no datagrams could be sent.
> > If less than the requested number of messages were sent, the application
> > must retry starting at the first failed one and if the problem is
> > persistent the error will be returned.
> > 
> > This matches the behaviour of other syscalls like read/write - it
> > is not an error if less than the requested number of elements are sent.
> 
> OK. David S. Miller suggested this behavior and Anton Blanchard agreed with
> this behavior.
> 
> Quoting from commit a2e27255 "net: Introduce recvmmsg socket syscall":
> | . R?mi Denis-Courmont & Steven Whitehouse: If we receive N < vlen
> |   datagrams and then recvmsg returns an error, recvmmsg will return
> |   the successfully received datagrams, store the error and return it
> |   in the next call.
> 
> R?mi Denis-Courmont, Steven Whitehouse and Arnaldo Carvalho de Melo, do you
> want to change recvmmsg()'s behaviour as well?

Since I've joined this part way through it seems, I'm assuming that if
something was sent/received then that will be returned and the error
stored until the next call. If nothing was sent/received then the error
can be returned immediately.

That is what I'd expect to be the case, since otherwise it is impossible
to know how much has been successfully sent/received in the partial
failure case, I think. Also it means that sendmmesg/recvmmsg matches
sendmsg/recvmsg in terms of expected return values and thus the
principle of least surprise.

So if thats what is being proposed, then it sounds good to me,

Steve.



^ permalink raw reply

* Re: return of ip_rt_bug()
From: Julian Anastasov @ 2011-08-05  7:56 UTC (permalink / raw)
  To: Tom London; +Cc: Dave Jones, netdev
In-Reply-To: <CAFiZG+UY+Rn=wqi6b9CM0CcARHJUGiqh_jpfEveGLhm+D2hNhA@mail.gmail.com>


	Hello,

On Thu, 4 Aug 2011, Tom London wrote:

> So, my router is just giving my wired NIC different addresses....
> 
> More I can provide?

	Thanks! Can you check in dmesg or in another log
about the first IP address in such line:

printk(KERN_DEBUG "ip_rt_bug: %pI4 -> %pI4, %s\n"

	Is it now 192.168.2.6 ?

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [RFC 4/4] [flexcan] Add support for FLEXCAN_DEBUG
From: Marc Kleine-Budde @ 2011-08-05  7:16 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-5-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 3634 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> Add a wrapper function for a register dump when a developer defines
> FLEXCAN_DEBUG

Comments inline..however I'm not sure if we need this patch.

> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |   40 ++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 40 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index fbb61c6..96684ca 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -316,6 +316,38 @@ static inline unsigned long flexcan_clk_get_rate(struct clk *clk)
>  
>  #endif
>  
> +#if defined(FLEXCAN_DEBUG)
> +void _flexcan_reg_dump(struct net_device *dev, const char *file, int line,
> +		       const char *func)
> +{
> +	const struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	printk(KERN_INFO "flexcan_reg_dump:%s:%d:%s()\n", file, line, func);

Use netdev_$LEVEL, please.
If you use dbg, you can remove the ifdef altogether.

> +	printk(KERN_INFO
> +		"\t  mcr 0x%08x  ctrl 0x%08x timer 0x%08x   rxg 0x%08x",
> +		flexcan_read(&regs->mcr),
> +		flexcan_read(&regs->ctrl),
> +		flexcan_read(&regs->timer),
> +		flexcan_read(&regs->rxgmask));
> +	printk(KERN_INFO
> +		"\t rx14 0x%08x  rx15 0x%08x   ecr 0x%08x   esr 0x%08x",
> +		flexcan_read(&regs->rx14mask),
> +		flexcan_read(&regs->rx15mask),
> +		flexcan_read(&regs->ecr),
> +		flexcan_read(&regs->esr));
> +	printk(KERN_INFO
> +		"\timsk2 0x%08x imsk1 0x%08x iflg2 0x%08x iflg1 0x%08x",
> +		flexcan_read(&regs->imask2),
> +		flexcan_read(&regs->imask1),
> +		flexcan_read(&regs->iflag2),
> +		flexcan_read(&regs->iflag1));
> +}
> +#define flexcan_reg_dump(_d) _flexcan_reg_dump(_d, __FILE__, __LINE__, __func__)
> +#else
> +#define flexcan_reg_dump(_d)
> +#endif
> +
>  /*
>   * Swtich transceiver on or off
>   */
> @@ -376,6 +408,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	u32 can_id;
>  	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (cf->can_dlc << 16);
>  
> +	flexcan_reg_dump(dev);
> +
>  	if (can_dropped_invalid_skb(dev, skb))
>  		return NETDEV_TX_OK;
>  
> @@ -408,6 +442,8 @@ static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  	/* tx_packets is incremented in flexcan_irq */
>  	stats->tx_bytes += cf->can_dlc;
>  
> +	flexcan_reg_dump(dev);
> +
>  	return NETDEV_TX_OK;
>  }
>  
> @@ -676,6 +712,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  	struct flexcan_regs __iomem *regs = priv->base;
>  	u32 reg_iflag1, reg_esr;
>  
> +	flexcan_reg_dump(dev);
> +
>  	reg_iflag1 = flexcan_read(&regs->iflag1);
>  	reg_esr = flexcan_read(&regs->esr);
>  	flexcan_write(FLEXCAN_ESR_ERR_INT, &regs->esr);	/* ACK err IRQ */
> @@ -716,6 +754,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>  		netif_wake_queue(dev);
>  	}
>  
> +	flexcan_reg_dump(dev);
> +
>  	return IRQ_HANDLED;
>  }
>  

cheers, Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [RFC 3/4] [flexcan] Add of_match to platform_device definition.
From: Marc Kleine-Budde @ 2011-08-05  7:13 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Wolfgang Grandegger
In-Reply-To: <1312509979-13226-4-git-send-email-holt-sJ/iWh9BUns@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1747 bytes --]

On 08/05/2011 04:06 AM, Robin Holt wrote:
> It looks like the of_device stuff got moved under the
> platform_device->driver and all we should need to do is define an of_match
> to get a flexcan_probe call out.

Please rephrase this commit message, at it goes into the git tree.
Otherwise the patche looks good.

Marc
> 
> Signed-off-by: Robin Holt <holt-sJ/iWh9BUns@public.gmane.org>
> To: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> To: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
> Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
>  drivers/net/can/flexcan.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 3417d0b..fbb61c6 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -1129,8 +1129,19 @@ static int __devexit flexcan_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +static struct of_device_id flexcan_of_match[] = {
> +	{
> +		.compatible = "fsl,flexcan",
> +	},
> +	{},
> +};
> +
>  static struct platform_driver flexcan_driver = {
> -	.driver.name = DRV_NAME,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = flexcan_of_match,
> +	},
>  	.probe = flexcan_probe,
>  	.remove = __devexit_p(flexcan_remove),
>  };


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [PATCH 2/3] net: Cap number of elements for sendmmsg
From: Tetsuo Handa @ 2011-08-05  5:50 UTC (permalink / raw)
  To: anton, acme, davem; +Cc: netdev, linux-security-module, eparis, casey, mjt
In-Reply-To: <20110805144619.729c631a@kryten>

Anton Blanchard wrote:
> > Apart from the upper limit for vlen argument of sendmmsg()/recvmmsg(),
> > we need to deal with stall problem (described below).
> 
> Capping vlen at 1024 should prevent that. Your patch does a signed
> comparison which just reduces the maximum value by 1 bit doesn't it?

Just for avoiding returning IS_ERR_VALUE() value upon success.

> Keep in mind each element could have up to 1024 iovec entries at worst
> case, so I think 1024 is a sane upper max.

OK. Please take Anton's version.

Arnaldo, please consider copying this change to recvmmsg() too.

^ permalink raw reply

* Re: [uclinux-dist-devel] [net-next v2 55/71] bfin_mac: Move the Analog Devices Inc driver
From: Bob Liu @ 2011-08-05  5:18 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, uclinux-dist-devel, sassmann, gospo
In-Reply-To: <1312082850-24914-56-git-send-email-jeffrey.t.kirsher@intel.com>

On Sun, Jul 31, 2011 at 11:27 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> Move the Analog Devices Inc driver into drivers/net/ethernet/adi/ and
> make the necessary Kconfig and Makefile changes.
>
> CC: <uclinux-dist-devel@blackfin.uclinux.org>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Looks good for me, Thanks.

Acked-by: Bob Liu <bob.liu@analog.com>

> ---
>  MAINTAINERS                               |    2 +-
>  drivers/net/Kconfig                       |   46 --------------------
>  drivers/net/Makefile                      |    1 -
>  drivers/net/ethernet/Kconfig              |    1 +
>  drivers/net/ethernet/Makefile             |    1 +
>  drivers/net/ethernet/adi/Kconfig          |   65 +++++++++++++++++++++++++++++
>  drivers/net/ethernet/adi/Makefile         |    5 ++
>  drivers/net/{ => ethernet/adi}/bfin_mac.c |    0
>  drivers/net/{ => ethernet/adi}/bfin_mac.h |    0
>  9 files changed, 73 insertions(+), 48 deletions(-)
>  create mode 100644 drivers/net/ethernet/adi/Kconfig
>  create mode 100644 drivers/net/ethernet/adi/Makefile
>  rename drivers/net/{ => ethernet/adi}/bfin_mac.c (100%)
>  rename drivers/net/{ => ethernet/adi}/bfin_mac.h (100%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a91e8fb..83a51ad 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1440,7 +1440,7 @@ BLACKFIN EMAC DRIVER
>  L:     uclinux-dist-devel@blackfin.uclinux.org
>  W:     http://blackfin.uclinux.org
>  S:     Supported
> -F:     drivers/net/bfin_mac.*
> +F:     drivers/net/ethernet/adi/
>
>  BLACKFIN RTC DRIVER
>  M:     Mike Frysinger <vapier.adi@gmail.com>
> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 61e71d0..bb4bc4b 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -252,52 +252,6 @@ config SH_ETH
>          This driver supporting CPUs are:
>                - SH7710, SH7712, SH7763, SH7619, SH7724, and SH7757.
>
> -config BFIN_MAC
> -       tristate "Blackfin on-chip MAC support"
> -       depends on NET_ETHERNET && (BF516 || BF518 || BF526 || BF527 || BF536 || BF537)
> -       select CRC32
> -       select MII
> -       select PHYLIB
> -       select BFIN_MAC_USE_L1 if DMA_UNCACHED_NONE
> -       help
> -         This is the driver for Blackfin on-chip mac device. Say Y if you want it
> -         compiled into the kernel. This driver is also available as a module
> -         ( = code which can be inserted in and removed from the running kernel
> -         whenever you want). The module will be called bfin_mac.
> -
> -config BFIN_MAC_USE_L1
> -       bool "Use L1 memory for rx/tx packets"
> -       depends on BFIN_MAC && (BF527 || BF537)
> -       default y
> -       help
> -         To get maximum network performance, you should use L1 memory as rx/tx buffers.
> -         Say N here if you want to reserve L1 memory for other uses.
> -
> -config BFIN_TX_DESC_NUM
> -       int "Number of transmit buffer packets"
> -       depends on BFIN_MAC
> -       range 6 10 if BFIN_MAC_USE_L1
> -       range 10 100
> -       default "10"
> -       help
> -         Set the number of buffer packets used in driver.
> -
> -config BFIN_RX_DESC_NUM
> -       int "Number of receive buffer packets"
> -       depends on BFIN_MAC
> -       range 20 100 if BFIN_MAC_USE_L1
> -       range 20 800
> -       default "20"
> -       help
> -         Set the number of buffer packets used in driver.
> -
> -config BFIN_MAC_USE_HWSTAMP
> -       bool "Use IEEE 1588 hwstamp"
> -       depends on BFIN_MAC && BF518
> -       default y
> -       help
> -         To support the IEEE 1588 Precision Time Protocol (PTP), select y here
> -
>  config NET_NETX
>        tristate "NetX Ethernet support"
>        select MII
> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> index 8e6dbd7..d249d76 100644
> --- a/drivers/net/Makefile
> +++ b/drivers/net/Makefile
> @@ -57,7 +57,6 @@ obj-$(CONFIG_EQUALIZER) += eql.o
>  obj-$(CONFIG_TUN) += tun.o
>  obj-$(CONFIG_VETH) += veth.o
>  obj-$(CONFIG_NET_NETX) += netx-eth.o
> -obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
>  obj-$(CONFIG_DM9000) += dm9000.o
>  obj-$(CONFIG_ENC28J60) += enc28j60.o
>  obj-$(CONFIG_ETHOC) += ethoc.o
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index 6b3b3dc..e087337 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -15,6 +15,7 @@ source "drivers/net/ethernet/3com/Kconfig"
>  source "drivers/net/ethernet/amd/Kconfig"
>  source "drivers/net/ethernet/apple/Kconfig"
>  source "drivers/net/ethernet/atheros/Kconfig"
> +source "drivers/net/ethernet/adi/Kconfig"
>  source "drivers/net/ethernet/broadcom/Kconfig"
>  source "drivers/net/ethernet/brocade/Kconfig"
>  source "drivers/net/ethernet/chelsio/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index 2a1cbce..826db27 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_NET_VENDOR_8390) += 8390/
>  obj-$(CONFIG_NET_VENDOR_AMD) += amd/
>  obj-$(CONFIG_NET_VENDOR_APPLE) += apple/
>  obj-$(CONFIG_NET_VENDOR_ATHEROS) += atheros/
> +obj-$(CONFIG_NET_BFIN) += adi/
>  obj-$(CONFIG_NET_VENDOR_BROADCOM) += broadcom/
>  obj-$(CONFIG_NET_VENDOR_BROCADE) += brocade/
>  obj-$(CONFIG_NET_VENDOR_CHELSIO) += chelsio/
> diff --git a/drivers/net/ethernet/adi/Kconfig b/drivers/net/ethernet/adi/Kconfig
> new file mode 100644
> index 0000000..a2c8a3b
> --- /dev/null
> +++ b/drivers/net/ethernet/adi/Kconfig
> @@ -0,0 +1,65 @@
> +#
> +# Blackfin device configuration
> +#
> +
> +config NET_BFIN
> +       bool "Blackfin devices"
> +       depends on BF516 || BF518 || BF526 || BF527 || BF536 || BF537
> +       ---help---
> +         If you have a network (Ethernet) card belonging to this class, say Y.
> +         Make sure you know the name of your card. Read the Ethernet-HOWTO,
> +         available from <http://www.tldp.org/docs.html#howto>.
> +
> +         If unsure, say Y.
> +
> +         Note that the answer to this question doesn't directly affect the
> +         kernel: saying N will just cause the configurator to skip all
> +         the remaining Blackfin card questions. If you say Y, you will be
> +         asked for your specific card in the following questions.
> +
> +config BFIN_MAC
> +       tristate "Blackfin on-chip MAC support"
> +       depends on NET_BFIN && (BF516 || BF518 || BF526 || BF527 || \
> +                BF536 || BF537)
> +       select CRC32
> +       select MII
> +       select PHYLIB
> +       select BFIN_MAC_USE_L1 if DMA_UNCACHED_NONE
> +       ---help---
> +         This is the driver for Blackfin on-chip mac device. Say Y if you want
> +         it compiled into the kernel. This driver is also available as a
> +         module ( = code which can be inserted in and removed from the running
> +         kernel whenever you want). The module will be called bfin_mac.
> +
> +config BFIN_MAC_USE_L1
> +       bool "Use L1 memory for rx/tx packets"
> +       depends on BFIN_MAC && (BF527 || BF537)
> +       default y
> +       ---help---
> +         To get maximum network performance, you should use L1 memory as rx/tx
> +         buffers. Say N here if you want to reserve L1 memory for other uses.
> +
> +config BFIN_TX_DESC_NUM
> +       int "Number of transmit buffer packets"
> +       depends on BFIN_MAC
> +       range 6 10 if BFIN_MAC_USE_L1
> +       range 10 100
> +       default "10"
> +       ---help---
> +         Set the number of buffer packets used in driver.
> +
> +config BFIN_RX_DESC_NUM
> +       int "Number of receive buffer packets"
> +       depends on BFIN_MAC
> +       range 20 100 if BFIN_MAC_USE_L1
> +       range 20 800
> +       default "20"
> +       ---help---
> +         Set the number of buffer packets used in driver.
> +
> +config BFIN_MAC_USE_HWSTAMP
> +       bool "Use IEEE 1588 hwstamp"
> +       depends on BFIN_MAC && BF518
> +       default y
> +       ---help---
> +         To support the IEEE 1588 Precision Time Protocol (PTP), select y here
> diff --git a/drivers/net/ethernet/adi/Makefile b/drivers/net/ethernet/adi/Makefile
> new file mode 100644
> index 0000000..b1fbe19
> --- /dev/null
> +++ b/drivers/net/ethernet/adi/Makefile
> @@ -0,0 +1,5 @@
> +#
> +# Makefile for the Blackfin device drivers.
> +#
> +
> +obj-$(CONFIG_BFIN_MAC) += bfin_mac.o
> diff --git a/drivers/net/bfin_mac.c b/drivers/net/ethernet/adi/bfin_mac.c
> similarity index 100%
> rename from drivers/net/bfin_mac.c
> rename to drivers/net/ethernet/adi/bfin_mac.c
> diff --git a/drivers/net/bfin_mac.h b/drivers/net/ethernet/adi/bfin_mac.h
> similarity index 100%
> rename from drivers/net/bfin_mac.h
> rename to drivers/net/ethernet/adi/bfin_mac.h
> --
> 1.7.6
>
> _______________________________________________
> Uclinux-dist-devel mailing list
> Uclinux-dist-devel@blackfin.uclinux.org
> https://blackfin.uclinux.org/mailman/listinfo/uclinux-dist-devel
>



-- 
Regards,
--Bob

^ permalink raw reply

* Re: [PATCH] Add pch ieee1588 driver for Intel EG20T PCH
From: Toshiharu Okada @ 2011-08-05  4:51 UTC (permalink / raw)
  To: Richard Cochran
  Cc: richard.cochran, netdev, linux-kernel, qi.wang, yong.y.wang,
	joel.clark, kok.howg.ewe, tomoya-linux
In-Reply-To: <20110803192331.GA11166@riccoc20.at.omicron.at>

Hi Richard

Thank you for your comments.
I will confirm them.

Toshiharu Okada
----- Original Message ----- 
From: "Richard Cochran" <richardcochran@gmail.com>
To: "Toshiharu Okada" <toshiharu-linux@dsn.okisemi.com>
Cc: <richard.cochran@omicron.at>; <netdev@vger.kernel.org>; 
<linux-kernel@vger.kernel.org>; <qi.wang@intel.com>; 
<yong.y.wang@intel.com>; <joel.clark@intel.com>; <kok.howg.ewe@intel.com>; 
<tomoya-linux@dsn.okisemi.com>
Sent: Thursday, August 04, 2011 4:23 AM
Subject: Re: [PATCH] Add pch ieee1588 driver for Intel EG20T PCH


On Wed, Aug 03, 2011 at 03:27:41PM +0900, Toshiharu Okada wrote:
> This patch is for IEEE1588 driver of Intel EG20T PCH.
> EG20T PCH is the platform controller hub that is used in Intel's
> general embedded platform.
> This driver adds support for using the EG20T PCH as a PTP clock.
>
> Would you review this patch, although this driver has not been tested yet?

Thanks for offering this patch. Please see my comments, below.

>
> Signed-off-by: Toshiharu Okada <toshiharu-linux@dsn.okisemi.com>
> ---
>  drivers/ptp/Kconfig   |   13 +
>  drivers/ptp/Makefile  |    1 +
>  drivers/ptp/ptp_pch.c |  629 
> +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 643 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/ptp/ptp_pch.c
>
> diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
> index 68d7201..cd9bc3b 100644
> --- a/drivers/ptp/Kconfig
> +++ b/drivers/ptp/Kconfig
> @@ -72,4 +72,17 @@ config DP83640_PHY
>    In order for this to work, your MAC driver must also
>    implement the skb_tx_timetamp() function.
>
> +config PTP_1588_CLOCK_PCH
> + tristate "Intel PCH EG20T as PTP clock"
> + depends on PTP_1588_CLOCK
> + depends on PCH_GBE
> + help
> +   This driver adds support for using the PCH EG20T as a PTP
> +   clock. This clock is only useful if your PTP programs are
> +   getting hardware time stamps on the PTP Ethernet packets
> +   using the SO_TIMESTAMPING API.
> +
> +   To compile this driver as a module, choose M here: the module
> +   will be called ptp_pch.
> +
>  endmenu
> diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
> index f6933e8..8b58597 100644
> --- a/drivers/ptp/Makefile
> +++ b/drivers/ptp/Makefile
> @@ -5,3 +5,4 @@
>  ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
>  obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
>  obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
> +obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o
> diff --git a/drivers/ptp/ptp_pch.c b/drivers/ptp/ptp_pch.c
> new file mode 100644
> index 0000000..0a804dc
> --- /dev/null
> +++ b/drivers/ptp/ptp_pch.c
> @@ -0,0 +1,629 @@
> +/*
> + * PTP 1588 clock using the EG20T PCH
> + *
> + * Copyright (C) 2010 OMICRON electronics GmbH
> + * Copyright (C) 2010 OKI SEMICONDUCTOR Co., LTD.
> + *
> + * This code was derived from the IXP46X driver.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307, 
> USA.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/ptp_clock_kernel.h>
> +
> +#define STATION_ADDR_LEN 20
> +#define PCI_DEVICE_ID_PCH_1588 0x8819
> +#define IO_MEM_BAR 1
> +
> +
> +/* Register read/write macros */
> +#define PCH_BIT_SET_CHECK(addr, bitmask) \
> + ((ioread32(addr) & (bitmask)) == (bitmask))
> +#define PCH_SET_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) | (bitmask)), (addr))
> +#define PCH_CLR_ADDR_BIT(addr, bitmask)\
> + iowrite32((ioread32(addr) & ~(bitmask)), (addr))

I don't really like macros of this sort, just to set and clear
register bits. It doesn't make any fewer lines nor does it clarify the
code. Every driver author will know what is meant by the plain old C,
like:

    x = x | mask;
    x = x & ~mask;

Also, you should consider whether you need to protect against
concurrent access on a register.

> +
> +#define DEFAULT_ADDEND 0xF0000029
> +#define TICKS_NS_SHIFT  4

This driver is based on my ixp46x driver, which is fine because,
looking at the data sheet, it appears that the time stamping unit in
the EG20T PCH is fairly similar.

However, I doubt that these ADDEND and SHIFT values will work for you,
since they were calculated for the frequency compensated clock on the
IXP. They reflect an input clock frequency of 66.666666 MHz and a
nominal frequency of 62.5 MHz (or a period of 16 nanoseconds, thus
SHIFT is 4).

Section 14.6.1.10 in the data sheet seems to imply that the input
clock is 50 MHz. In that case you could use a nominal frequency of
31.25 MHz (period 32 ns, SHIFT 5). However, you need to find out
the actual input clock frequency. If this can vary, then the driver
should allow changing these values.

> +#define N_EXT_TS 2
> +
> +enum pch_status {
> + PCH_SUCCESS,
> + PCH_INVALIDPARAM,
> + PCH_NOTIMESTAMP,
> + PCH_INTERRUPTMODEINUSE,
> + PCH_FAILED,
> + PCH_UNSUPPORTED,
> +};
> +/**
> + * struct pch_ts_regs - IEEE 1588 registers
> + */
> +struct pch_ts_regs {
> + u32 control;
> + u32 event;
> + u32 addend;
> + u32 accum;
> + u32 test;
> + u32 ts_compare;
> + u32 rsystime_lo;
> + u32 rsystime_hi;
> + u32 systime_lo;
> + u32 systime_hi;
> + u32 trgt_lo;
> + u32 trgt_hi;
> + u32 asms_lo;
> + u32 asms_hi;
> + u32 amms_lo;
> + u32 amms_hi;
> + u32 ch_control;

You never program this register. But I think the "Mode" field should
be set to 2. The other settings really make no sense at all.  Or do
you set this in the MAC driver?

(I wonder why Intel retained the very limited PTP V1 modes from the
IXP time stamping unit.)

> + u32 ch_event;
> + u32 tx_snap_lo;
> + u32 tx_snap_hi;
> + u32 rx_snap_lo;
> + u32 rx_snap_hi;
> + u32 src_uuid_lo;
> + u32 src_uuid_hi;
> + u32 can_status;
> + u32 can_snap_lo;
> + u32 can_snap_hi;
> + u32 ts_sel;
> + u32 ts_st[6];
> + u32 reserve1[15];
> + u32 stl_max_set_en;
> + u32 stl_max_set;
> + u32 reserve2[13];
> + u32 srst;
> +};
> +
> +#define PCH_TSC_RESET (1 << 0)
> +#define PCH_TSC_TTM_MASK (1 << 1)
> +#define PCH_TSC_ASMS_MASK (1 << 2)
> +#define PCH_TSC_AMMS_MASK (1 << 3)
> +#define PCH_TSC_PPSM_MASK (1 << 4)
> +#define PCH_TSE_TTIPEND (1 << 1)
> +#define PCH_TSE_SNS (1 << 2)
> +#define PCH_TSE_SNM (1 << 3)
> +#define PCH_TSE_PPS (1 << 4)
> +#define PCH_CC_MM (1 << 0)
> +#define PCH_CC_TA (1 << 1)
> +
> +#define PCH_CC_MODE_SHIFT 16
> +#define PCH_CC_MODE_MASK 0x001F0000
> +#define PCH_CC_VERSION (1 << 31)
> +#define PCH_CE_TXS (1 << 0)
> +#define PCH_CE_RXS (1 << 1)
> +#define PCH_CE_OVR (1 << 0)
> +#define PCH_CE_VAL (1 << 1)
> +#define PCH_ECS_ETH (1 << 0)
> +
> +#define PCH_ECS_CAN (1 << 1)
> +#define PCH_STATION_BYTES 6
> +
> +#define PCH_IEEE1588_ETH (1 << 0)
> +#define PCH_IEEE1588_CAN (1 << 1)
> +/**
> + * struct pch_dev - Driver private data
> + */
> +struct pch_dev {
> + struct pch_ts_regs *regs;
> + struct ptp_clock *ptp_clock;
> + struct ptp_clock_info caps;
> + int exts0_enabled;
> + int exts1_enabled;
> +
> + u32 mem_base;
> + u32 mem_size;
> + u32 irq;
> + u32 suspend:1;
> + u32 initialized:1;
> + struct pci_dev *pdev;
> + spinlock_t lock;

It would be nice to have a short comment telling what this lock
protects. (Yes, I know what it protects, but still that is good
practice to have a comment.)

> +};
> +
> +static inline void pch_eth_enable_set(struct pch_dev *chip)
> +{
> + /* SET the eth_enable bit */
> + PCH_SET_ADDR_BIT(&chip->regs->ts_sel, PCH_ECS_ETH);
> +}

I really don't like this or the other similar helper functions,
below. They don't make the driver more understandable or shorter.
This function is only used in one place. You need at least two callers
to justify this.

> +
> +/*
> + * Register access functions
> + */
> +
> +static u64 pch_systime_read(struct pch_ts_regs *regs)
> +{
> + u64 ns;
> + u32 lo, hi;
> +
> + lo = ioread32(&regs->systime_lo);
> + hi = ioread32(&regs->systime_hi);
> +
> + ns = ((u64) hi) << 32;
> + ns |= lo;
> + ns <<= TICKS_NS_SHIFT;

Here 'ns' will only be in nanoseconds when the ADDEND and SHIFT macros
are correct for the actual machine, as I mentioned above.

> +
> + return ns;
> +}
> +
> +static void pch_systime_write(struct pch_ts_regs *regs, u64 ns)
> +{
> + u32 hi, lo;
> +
> + ns >>= TICKS_NS_SHIFT;
> + hi = ns >> 32;
> + lo = ns & 0xffffffff;
> +
> + iowrite32(lo, &regs->systime_lo);
> + iowrite32(hi, &regs->systime_hi);
> +}
> +

I think the following helper functions ...

> +static inline u32 pch_pps_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for PPS event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline u32 pch_amms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Master Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline u32 pch_asms_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Auxiliary Slave Mode Snapshot Captured event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline u32 pch_ttm_evt_get(struct pch_dev *chip)
> +{
> + /* Poll for Target Time Reached event */
> + return PCH_BIT_SET_CHECK(&chip->regs->event, PCH_TSE_TTIPEND);
> +}
> +
> +static inline void pch_pps_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear PPS event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_PPS);
> +}
> +
> +static inline void pch_amms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Master Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNM);
> +}
> +
> +static inline void pch_asms_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Auxiliary Slave Mode Snapshot Captured event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_SNS);
> +}
> +
> +static inline void pch_ttm_evt_clear(struct pch_dev *chip)
> +{
> + /* Clear Target Time Reached event */
> + PCH_SET_ADDR_BIT(&chip->regs->event, PCH_TSE_TTIPEND);
> +}

... are just noise and add nothing useful to the driver.

> +
> +static inline void pch_block_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist block */
> + PCH_SET_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> + PCH_CLR_ADDR_BIT(&chip->regs->control, PCH_TSC_RESET);
> +}
> +
> +/* This function enables all 64 bits in system time registers [high & 
> low].
> +This is a work-around for non continuous value in the SystemTime 
> Register*/
> +static void pch_set_system_time_count(struct pch_dev *chip)
> +{
> + iowrite32(0x01, &chip->regs->stl_max_set_en);
> + iowrite32(0xFFFFFFFF, &chip->regs->stl_max_set);
> + iowrite32(0x00, &chip->regs->stl_max_set_en);
> +}
> +
> +static void pch_reset(struct pch_dev *chip)
> +{
> + /* Reset Hardware Assist */
> + pch_block_reset(chip);
> +
> + /* enable all 32 bits in system time registers */
> + pch_set_system_time_count(chip);
> +}

These three, above, are okay, since they encapsulate one logical
operation that takes more than one register access.

However, you might consider whether you need locking here.

> +
> +static void pch_eth_enable(struct pch_dev *chip)
> +{
> + pch_eth_enable_set(chip);
> +}

Again, this helper only has one caller. Why not just set the bit that
you need in line?

> +
> +/*
> + * Interrupt service routine
> + */
> +static irqreturn_t isr(int irq, void *priv)
> +{
> + struct pch_dev *pch_dev = priv;
> + struct pch_ts_regs *regs = pch_dev->regs;
> + struct ptp_clock_event event;
> + u32 ack = 0, lo, hi, val;
> +
> + val = ioread32(&regs->event);
> +
> + if (val & PCH_TSE_SNS) {
> + ack |= PCH_TSE_SNS;
> + if (pch_dev->exts0_enabled) {
> + hi = ioread32(&regs->asms_hi);
> + lo = ioread32(&regs->asms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 0;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_SNM) {
> + ack |= PCH_TSE_SNM;
> + if (pch_dev->exts1_enabled) {
> + hi = ioread32(&regs->amms_hi);
> + lo = ioread32(&regs->amms_lo);
> + event.type = PTP_CLOCK_EXTTS;
> + event.index = 1;
> + event.timestamp = ((u64) hi) << 32;
> + event.timestamp |= lo;
> + event.timestamp <<= TICKS_NS_SHIFT;
> + ptp_clock_event(pch_dev->ptp_clock, &event);
> + }
> + }
> +
> + if (val & PCH_TSE_TTIPEND)
> + ack |= PCH_TSE_TTIPEND; /* this bit seems to be always set */

This ISR code (and much of the rest of the driver) is copied from the
IXP driver. It would be nice to know if it actually works on the atom
hardware. Do have any hardware to try it on?

> +
> + if (ack) {
> + iowrite32(ack, &regs->event);
> + return IRQ_HANDLED;
> + } else
> + return IRQ_NONE;
> +}
> +
> +/*
> + * PTP clock operations
> + */
> +
> +static int ptp_pch_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
> +{
> + u64 adj;
> + u32 diff, addend;
> + int neg_adj = 0;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + if (ppb < 0) {
> + neg_adj = 1;
> + ppb = -ppb;
> + }
> + addend = DEFAULT_ADDEND;
> + adj = addend;
> + adj *= ppb;
> + diff = div_u64(adj, 1000000000ULL);
> +
> + addend = neg_adj ? addend - diff : addend + diff;
> +
> + iowrite32(addend, &regs->addend);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_adjtime(struct ptp_clock_info *ptp, s64 delta)
> +{
> + s64 now;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + now = pch_systime_read(regs);
> + now += delta;
> + pch_systime_write(regs, now);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_gettime(struct ptp_clock_info *ptp, struct timespec 
> *ts)
> +{
> + u64 ns;
> + u32 remainder;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + ns = pch_systime_read(regs);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + ts->tv_sec = div_u64_rem(ns, 1000000000, &remainder);
> + ts->tv_nsec = remainder;
> + return 0;
> +}
> +
> +static int ptp_pch_settime(struct ptp_clock_info *ptp,
> +    const struct timespec *ts)
> +{
> + u64 ns;
> + unsigned long flags;
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> + struct pch_ts_regs *regs = pch_dev->regs;
> +
> + ns = ts->tv_sec * 1000000000ULL;
> + ns += ts->tv_nsec;
> +
> + spin_lock_irqsave(&pch_dev->lock, flags);
> + pch_systime_write(regs, ns);
> + spin_unlock_irqrestore(&pch_dev->lock, flags);
> +
> + return 0;
> +}
> +
> +static int ptp_pch_enable(struct ptp_clock_info *ptp,
> +   struct ptp_clock_request *rq, int on)
> +{
> + struct pch_dev *pch_dev = container_of(ptp, struct pch_dev, caps);
> +
> + switch (rq->type) {
> + case PTP_CLK_REQ_EXTTS:
> + switch (rq->extts.index) {
> + case 0:
> + pch_dev->exts0_enabled = on ? 1 : 0;
> + break;
> + case 1:
> + pch_dev->exts1_enabled = on ? 1 : 0;
> + break;
> + default:
> + return -EINVAL;
> + }
> + return 0;
> + default:
> + break;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static struct ptp_clock_info ptp_pch_caps = {
> + .owner = THIS_MODULE,
> + .name = "PCH timer",
> + .max_adj = 66666655,

This should be recalculated once you figure out the input clock and
nominal frequency.

> + .n_ext_ts = N_EXT_TS,
> + .pps = 0,
> + .adjfreq = ptp_pch_adjfreq,
> + .adjtime = ptp_pch_adjtime,
> + .gettime = ptp_pch_gettime,
> + .settime = ptp_pch_settime,
> + .enable = ptp_pch_enable,
> +};
> +
> +
> +#ifdef CONFIG_PM
> +static s32 pch_suspend(struct pci_dev *pdev, pm_message_t state)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + chip->suspend = 1;

You set this flag here ...

> + pci_disable_device(pdev);
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> +
> + if (pci_save_state(pdev) != 0) {
> + dev_err(&pdev->dev,
> + "%s: could not save PCI config state\n", __func__);
> + return -ENOMEM;
> + }
> + pci_set_power_state(pdev, pci_choose_state(pdev, state));
> +
> + return 0;
> +}
> +
> +static s32 pch_resume(struct pci_dev *pdev)
> +{
> + s32 ret;
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + pci_set_power_state(pdev, PCI_D0);
> + pci_restore_state(pdev);
> + ret = pci_enable_device(pdev);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: pci_enable_device failed\n", __func__);
> + return ret;
> + }
> + pci_enable_wake(pdev, PCI_D3hot, 0);
> + chip->suspend = 0;

... and clear it again here. Why?

> + return 0;
> +}
> +#else
> +#define pch_suspend NULL
> +#define pch_resume NULL
> +#endif
> +
> +static void __devexit pch_remove(struct pci_dev *pdev)
> +{
> + struct pch_dev *chip = pci_get_drvdata(pdev);
> +
> + ptp_clock_unregister(chip->ptp_clock);
> + /* free the interrupt */
> + if (pdev->irq != 0)
> + free_irq(pdev->irq, chip);
> +
> + /* unmap the virtual IO memory space */
> + if (chip->regs != 0) {
> + iounmap(chip->regs);
> + chip->regs = 0;
> + }
> + /* release the reserved IO memory space */
> + if (chip->mem_base != 0) {
> + release_mem_region(chip->mem_base, chip->mem_size);
> + chip->mem_base = 0;
> + }
> + pci_disable_device(pdev);
> + kfree(chip);
> + dev_info(&pdev->dev, "%s: complete\n", __func__);
> +}
> +
> +static s32 __devinit
> +pch_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> +{
> + s32 ret;
> + struct pch_dev *chip;
> +
> + chip = kzalloc(sizeof(struct pch_dev), GFP_KERNEL);
> + if (chip == NULL)
> + return -ENOMEM;
> +
> + /* enable the 1588 pci device */
> + ret = pci_enable_device(pdev);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s:could not enable the pci device\n", __func__);
> + goto err_pci_en;
> + }
> +
> + chip->mem_base = pci_resource_start(pdev, IO_MEM_BAR);
> + if (!chip->mem_base) {
> + dev_err(&pdev->dev,
> + "%s: could not locate IO memory address\n", __func__);
> + ret = -ENODEV;
> + goto err_pci_start;
> + }
> +
> + /* retreive the available length of the IO memory space */
> + chip->mem_size = pci_resource_len(pdev, IO_MEM_BAR);
> +
> + /* allocate the memory for the device registers */
> + if (!request_mem_region
> +     (chip->mem_base, chip->mem_size, "1588_regs")) {

Poor statement break (and this would fit all on one line).

> + dev_err(&pdev->dev,
> +     "%s: could not allocate register memory space\n", __func__);

Bad indentation.

> + ret = -EBUSY;
> + goto err_req_mem_region;
> + }
> +
> + /* get the virtual address to the 1588 registers */
> + chip->regs = ioremap(chip->mem_base, chip->mem_size);
> +
> + if (!chip->regs) {
> + dev_err(&pdev->dev,
> + "%s: Could not get virtual address\n", __func__);
> + ret = -ENOMEM;
> + goto err_ioremap;
> + }
> +
> + chip->caps = ptp_pch_caps;
> + chip->ptp_clock = ptp_clock_register(&chip->caps);
> +
> + if (IS_ERR(chip->ptp_clock))
> + return PTR_ERR(chip->ptp_clock);
> +
> + spin_lock_init(&chip->lock);
> +
> + ret = request_irq(pdev->irq, &isr, IRQF_SHARED, KBUILD_MODNAME, chip);
> + if (ret != 0) {
> + dev_err(&pdev->dev,
> + "%s: failed to get irq %d\n", __func__, pdev->irq);
> + goto err_req_irq;
> + }
> +
> + chip->initialized = 1;

You set this flag, but never use it.

> + /* indicate success */
> + chip->irq = pdev->irq;
> + chip->pdev = pdev;
> + pci_set_drvdata(pdev, chip);
> +
> + /* reset the ieee1588 h/w */
> + pch_reset(chip);
> +
> + iowrite32(DEFAULT_ADDEND, &chip->regs->addend);
> + iowrite32(1, &chip->regs->trgt_lo);
> + iowrite32(0, &chip->regs->trgt_hi);
> + iowrite32(PCH_TSE_TTIPEND, &chip->regs->event);
> + pch_eth_enable(chip);
> +
> + return 0;
> +
> +err_req_irq:
> + ptp_clock_unregister(chip->ptp_clock);
> + iounmap(chip->regs);
> + chip->regs = 0;
> +
> +err_ioremap:
> + release_mem_region(chip->mem_base, chip->mem_size);
> +
> +err_req_mem_region:
> + chip->mem_base = 0;
> +
> +err_pci_start:
> + pci_disable_device(pdev);
> +
> +err_pci_en:
> + kfree(chip);
> + dev_err(&pdev->dev, "%s: probe failed(ret=0x%x)\n", __func__, ret);
> +
> + return ret;
> +}
> +
> +static DEFINE_PCI_DEVICE_TABLE(pch_gbe_pcidev_id) = {
> + {.vendor = PCI_VENDOR_ID_INTEL,

Needs a space (or newline) before the dot.

> + .device = PCI_DEVICE_ID_PCH_1588
> + },
> + {0}
> +};
> +
> +static struct pci_driver pch_pcidev = {
> + .name = KBUILD_MODNAME,
> + .id_table = pch_pcidev_id,

Here you meant "pch_gbe_pcidev_id" instead (or no "gbe" in the PCI
device table).

> + .probe = pch_probe,
> + .remove = pch_remove,
> + .suspend = pch_suspend,
> + .resume = pch_resume,
> +};
> +
> +static void __exit ptp_pch_exit(void)
> +{
> + pci_unregister_driver(&pch_pcidev);
> +}
> +
> +static s32 __init ptp_pch_init(void)
> +{
> + s32 ret;
> +
> + /* register the driver with the pci core */
> + ret = pci_register_driver(&pch_pcidev);
> +
> + return ret;
> +}
> +
> +module_init(ptp_pch_init);
> +module_exit(ptp_pch_exit);
> +
> +MODULE_AUTHOR("OKI SEMICONDUCTOR, <toshiharu-linux@dsn.okisemi.com>");
> +MODULE_DESCRIPTION("PTP clock using the EG20T timer");
> +MODULE_LICENSE("GPL");

Overall, the driver looks okay. I would appreciate if you would take a
look at the comments and submit a revised patch.

I would also like to see how the time stamps are done in the MAC
driver. Have you already posted that?

Feature request: I notice in the data sheet that the time stamping
unit can produce a PPS output. Any chance that you could program this
feature?

Thanks,

Richard


> +
> -- 
> 1.7.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* Re: [PATCH 2/3] net: Cap number of elements for sendmmsg
From: Anton Blanchard @ 2011-08-05  4:46 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: acme, netdev, linux-security-module, davem, eparis, casey, mjt
In-Reply-To: <201108050429.p754TTBa030939@www262.sakura.ne.jp>


Hi,

> I think 1024 is a reasonable value.
> But I also worry that programmers may wish to send more.
> 
> 
> Apart from the upper limit for vlen argument of sendmmsg()/recvmmsg(),
> we need to deal with stall problem (described below).

Capping vlen at 1024 should prevent that. Your patch does a signed
comparison which just reduces the maximum value by 1 bit doesn't it?

Keep in mind each element could have up to 1024 iovec entries at worst
case, so I think 1024 is a sane upper max.

Anton

> It may be possible to abuse sendmmsg() which was introduced in Linux
> 3.0 and recvmmsg() which was introduced in Linux 2.6.33 for
> triggering CPU stall warning.
> 
> I ran below program
> 
> ---------- Test program start ----------
> #include <string.h>
> #include <stdlib.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <netdb.h>
> #include <sys/types.h>
> #include <sys/ioctl.h>
> #include <sys/socket.h>
> #include <asm/unistd.h>
> #include <errno.h>
> 
> #ifndef __NR_sendmmsg
> #if defined( __PPC__)
> #define __NR_sendmmsg	349
> #elif defined(__x86_64__)
> #define __NR_sendmmsg	307
> #elif defined(__i386__)
> #define __NR_sendmmsg	345
> #else
> #error __NR_sendmmsg not defined
> #endif
> #endif
> 
> struct mmsghdr {
> 	struct msghdr msg_hdr;
> 	unsigned int msg_len;
> };
> 
> static inline int sendmmsg(int fd, struct mmsghdr *mmsg, unsigned
> vlen, unsigned flags)
> {
> 	return syscall(__NR_sendmmsg, fd, mmsg, vlen, flags, NULL);
> }
> 
> #define NUMBATCH (1048576 * 64)
> 
> int main(int argc, char *argv[])
> {
> 	const int fd = socket(AF_INET, SOCK_DGRAM, 0);
> 	struct mmsghdr *datagrams;
> 	unsigned int i;
> 	struct iovec iovec = { };
> 	struct sockaddr_in addr = { };
> 	addr.sin_family = AF_INET;
> 	addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> 	addr.sin_port = htons(10000);
> 	datagrams = calloc(sizeof(*datagrams), NUMBATCH);
> 	for (i = 0; i < NUMBATCH; ++i) {
> 		datagrams[i].msg_hdr.msg_iov = &iovec;
> 		datagrams[i].msg_hdr.msg_iovlen = 1;
> 		datagrams[i].msg_hdr.msg_name = &addr;
> 		datagrams[i].msg_hdr.msg_namelen = sizeof(addr);
> 	}
> 	printf("Calling sendmmsg()\n");
> 	printf("%d\n", sendmmsg(fd, datagrams, NUMBATCH, 0));
> 	printf("Done\n");
> 	return 0;
> }
> ---------- Test program end ----------
> 
> and got below output.
> 
> # time ./a.out
> Calling sendmmsg()
> INFO: rcu_sched_state detected stall on CPU 0 (t=15000 jiffies)
> 67108864
> Done
> 
> real    2m48.736s
> user    0m0.128s
> sys     0m16.489s
> 
> 
> 
> If this application created threads that matches number of CPUs
> available, and entered into sendmmsg(), the machine will hang for
> many seconds.
> 
> Also, signals are ignored when this application is in sendmmsg().
> That is, if this application is holding much RAM (like above program)
> and is selected by OOM-killer, this application cannot be killed
> until returns from sendmmsg().
> 
> Applying below patch solves the stall message and signal delaying
> problem. ----------------------------------------
> [PATCH] net: Fix sendmmsg() stall problem.
> 
> If the caller passed a huge value (e.g. 64M) to vlen argument of
> sendmmsg(), the caller triggers "INFO: rcu_sched_state detected stall
> on CPU X" message because there is no chance to call scheduler. Thus
> give a chance to call scheduler and also check for pending signal.
> 
> Also, if the caller passed a value where IS_ERR() returns true (e.g.
> UINT_MAX), the caller will get EOF and errno will be set to (e.g.)
> EPERM when all datagrams are successfully sent. Thus, limit the max
> value of vlen to INT_MAX.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: stable <stable@kernel.org> [3.0+]
> ---
>  net/socket.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- linux-3.0.orig/net/socket.c
> +++ linux-3.0/net/socket.c
> @@ -1999,6 +1999,8 @@ int __sys_sendmmsg(int fd, struct mmsghd
>  	struct compat_mmsghdr __user *compat_entry;
>  	struct msghdr msg_sys;
>  
> +	if ((int) vlen < 0)
> +		return -EINVAL;
>  	datagrams = 0;
>  
>  	sock = sockfd_lookup_light(fd, &err, &fput_needed);
> @@ -2035,6 +2037,9 @@ int __sys_sendmmsg(int fd, struct mmsghd
>  		if (err)
>  			break;
>  		++datagrams;
> +		cond_resched();
> +		if (signal_pending(current))
> +			break;
>  	}
>  
>  out_put:
> ----------------------------------------
> 
> The situation is similar regaring recvmmsg(). Although recvmmsg()
> less likely stalls than sendmmsg() does, it could happen if a huge
> number of datagrams are in the socket's receive queue and the caller
> attempted to fetch all of them at once. Thus, we may want below patch
> as well.
> 
> ----------------------------------------
> [PATCH] net: Fix recvmmsg() stall problem.
> 
> If the caller passed a huge value to vlen argument of recvmmsg() and
> there are enough datagrams in the socket's receive queue, trying to
> pick up all at once may trigger "INFO: rcu_sched_state detected stall
> on CPU X" message because there is no chance to call scheduler. Thus
> give a chance to call scheduler and also check for pending signal.
> 
> Also, if the caller passed a value where IS_ERR() returns true (e.g.
> UINT_MAX), the caller will get EOF and errno will be set to (e.g.)
> EPERM when all datagrams are successfully received. Thus, limit the
> max value of vlen to INT_MAX.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Cc: stable <stable@kernel.org> [2.6.33+]
> ---
>  net/socket.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> --- linux-3.0.orig/net/socket.c
> +++ linux-3.0/net/socket.c
> @@ -2204,6 +2204,8 @@ int __sys_recvmmsg(int fd, struct mmsghd
>  	struct msghdr msg_sys;
>  	struct timespec end_time;
>  
> +	if ((int) vlen < 0)
> +		return -EINVAL;
>  	if (timeout &&
>  	    poll_select_set_timeout(&end_time, timeout->tv_sec,
>  				    timeout->tv_nsec))
> @@ -2247,6 +2249,9 @@ int __sys_recvmmsg(int fd, struct mmsghd
>  		if (err)
>  			break;
>  		++datagrams;
> +		cond_resched();
> +		if (signal_pending(current))
> +			break;
>  
>  		/* MSG_WAITFORONE turns on MSG_DONTWAIT after one
> packet */ if (flags & MSG_WAITFORONE)
> ----------------------------------------
> 


^ permalink raw reply

* Re: [net-next-2.6 PATCH] enic: Add timestamp to network interface stats
From: Stephen Hemminger @ 2011-08-05  4:42 UTC (permalink / raw)
  To: Danny Guo; +Cc: davem, netdev
In-Reply-To: <20110805001124.32402.42919.stgit@savbu-pc100.cisco.com>

On Thu, 04 Aug 2011 17:11:24 -0700
Danny Guo <dannguo@cisco.com> wrote:

> From: Danny Guo <dannguo@cisco.com>
> 
> This patch adds timestamps in ethtool stats. It makes it easier to provide scripts to users to calculate throughput, etc. It also allows software to synchronize timestamps with host time for correlating host events with stats collection.
> 
> Signed-off-by: Danny Guo <dannguo@cisco.com>
> Signed-off-by: Vasanthy Kolluri <vkolluri@cisco.com>
> Signed-off-by: Roopa Prabhu <roprabhu@cisco.com>
> Signed-off-by: David Wang <dwang2@cisco.com>


The concept is okay, but don't like drivers doing device specific
things all the time.

> ---
>  drivers/net/enic/enic.h       |    2 +-
>  drivers/net/enic/enic_main.c  |    7 ++++++-
>  drivers/net/enic/vnic_dev.c   |    1 +
>  drivers/net/enic/vnic_stats.h |    1 +
>  4 files changed, 9 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
> index ce76d9a..2e58bdc 100644
> --- a/drivers/net/enic/enic.h
> +++ b/drivers/net/enic/enic.h
> @@ -32,7 +32,7 @@
>  
>  #define DRV_NAME		"enic"
>  #define DRV_DESCRIPTION		"Cisco VIC Ethernet NIC Driver"
> -#define DRV_VERSION		"2.1.1.24"
> +#define DRV_VERSION		"2.1.1.25"
>  #define DRV_COPYRIGHT		"Copyright 2008-2011 Cisco Systems, Inc"
>  
>  #define ENIC_BARS_MAX		6
> diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
> index 67a27cd..a3f61c1 100644
> --- a/drivers/net/enic/enic_main.c
> +++ b/drivers/net/enic/enic_main.c
> @@ -121,6 +121,8 @@ static const struct enic_stat enic_rx_stats[] = {
>  static const unsigned int enic_n_tx_stats = ARRAY_SIZE(enic_tx_stats);
>  static const unsigned int enic_n_rx_stats = ARRAY_SIZE(enic_rx_stats);
>  
> +#define ENIC_TIMESTAMP_STAT_NAME	"timestamp(ns)"
> +
>  static int enic_is_dynamic(struct enic *enic)
>  {
>  	return enic->pdev->device == PCI_DEVICE_ID_CISCO_VIC_ENET_DYN;
> @@ -224,6 +226,8 @@ static void enic_get_strings(struct net_device *netdev, u32 stringset, u8 *data)
>  			memcpy(data, enic_rx_stats[i].name, ETH_GSTRING_LEN);
>  			data += ETH_GSTRING_LEN;
>  		}
> +		memcpy(data, ENIC_TIMESTAMP_STAT_NAME, ETH_GSTRING_LEN);
> +		data += ETH_GSTRING_LEN;
>  		break;
>  	}
>  }
> @@ -232,7 +236,7 @@ static int enic_get_sset_count(struct net_device *netdev, int sset)
>  {
>  	switch (sset) {
>  	case ETH_SS_STATS:
> -		return enic_n_tx_stats + enic_n_rx_stats;
> +		return enic_n_tx_stats + enic_n_rx_stats + 1;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
> @@ -251,6 +255,7 @@ static void enic_get_ethtool_stats(struct net_device *netdev,
>  		*(data++) = ((u64 *)&vstats->tx)[enic_tx_stats[i].offset];
>  	for (i = 0; i < enic_n_rx_stats; i++)
>  		*(data++) = ((u64 *)&vstats->rx)[enic_rx_stats[i].offset];
> +	*(data++) = vstats->timestamp_ns;
>  }
>  
>  static u32 enic_get_msglevel(struct net_device *netdev)
> diff --git a/drivers/net/enic/vnic_dev.c b/drivers/net/enic/vnic_dev.c
> index 8c4c8cf..656871a 100644
> --- a/drivers/net/enic/vnic_dev.c
> +++ b/drivers/net/enic/vnic_dev.c
> @@ -467,6 +467,7 @@ int vnic_dev_stats_dump(struct vnic_dev *vdev, struct vnic_stats **stats)
>  		if (!vdev->stats)
>  			return -ENOMEM;
>  	}
> +	memset(vdev->stats, 0, sizeof(*vdev->stats));

No, reading stats should not clear them!


>  	*stats = vdev->stats;
>  	a0 = vdev->stats_pa;
> diff --git a/drivers/net/enic/vnic_stats.h b/drivers/net/enic/vnic_stats.h
> index 77750ec..9d5158d 100644
> --- a/drivers/net/enic/vnic_stats.h
> +++ b/drivers/net/enic/vnic_stats.h
> @@ -65,6 +65,7 @@ struct vnic_rx_stats {
>  struct vnic_stats {
>  	struct vnic_tx_stats tx;
>  	struct vnic_rx_stats rx;
> +	u64 timestamp_ns;
>  };

Two problems: one is that you are exposing a timestamp in a random unit, and
the other problem is that is never set (at least in this code).


^ 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