Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 01/10] can: Remove extern from function prototypes
From: Marc Kleine-Budde @ 2013-09-24  7:37 UTC (permalink / raw)
  To: Joe Perches
  Cc: netdev, David S. Miller, linux-kernel, Wolfgang Grandegger,
	linux-can
In-Reply-To: <5570169a078375fa8662adeb2a7f24c1ae718bfb.1379974101.git.joe@perches.com>

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

On 09/24/2013 12:11 AM, Joe Perches wrote:
> There are a mix of function prototypes with and without extern
> in the kernel sources.  Standardize on not using extern for
> function prototypes.
> 
> Function prototypes don't need to be written with extern.
> extern is assumed by the compiler.  Its use is as unnecessary as
> using auto to declare automatic/local variables in a block.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Thx, added to linux-can-next. The patch will be included in the next
pull request to David.

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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* [PATCH] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Aida Mynzhasova @ 2013-09-24  7:39 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree, netdev

Currently IEEE 1588 timer reference clock source is determined through
hard-coded value in gianfar_ptp driver. This patch allows to select ptp
clock source by means of device tree file node.

For instance:

	fsl,cksel = <0>;

for using external (TSEC_TMR_CLK input) high precision timer
reference clock.

Other acceptable values:

	<1> : eTSEC system clock
	<2> : eTSEC1 transmit clock
	<3> : RTC clock input

When this attribute isn't used, eTSEC system clock will serve as
IEEE 1588 timer reference clock.

Signed-off-by: Aida Mynzhasova <aida.mynzhasova@skitlab.ru>
---
 Documentation/devicetree/bindings/net/fsl-tsec-phy.txt | 2 ++
 drivers/net/ethernet/freescale/gianfar_ptp.c           | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
index 2c6be03..2f889f1 100644
--- a/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
+++ b/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt
@@ -86,6 +86,7 @@ General Properties:
 
 Clock Properties:
 
+  - fsl,cksel        Timer reference clock source.
   - fsl,tclk-period  Timer reference clock period in nanoseconds.
   - fsl,tmr-prsc     Prescaler, divides the output clock.
   - fsl,tmr-add      Frequency compensation value.
@@ -121,6 +122,7 @@ Example:
 		reg = <0x24E00 0xB0>;
 		interrupts = <12 0x8 13 0x8>;
 		interrupt-parent = < &ipic >;
+		fsl,cksel       = <1>;
 		fsl,tclk-period = <10>;
 		fsl,tmr-prsc    = <100>;
 		fsl,tmr-add     = <0x999999A4>;
diff --git a/drivers/net/ethernet/freescale/gianfar_ptp.c b/drivers/net/ethernet/freescale/gianfar_ptp.c
index 098f133..e006a09 100644
--- a/drivers/net/ethernet/freescale/gianfar_ptp.c
+++ b/drivers/net/ethernet/freescale/gianfar_ptp.c
@@ -452,7 +452,9 @@ static int gianfar_ptp_probe(struct platform_device *dev)
 	err = -ENODEV;
 
 	etsects->caps = ptp_gianfar_caps;
-	etsects->cksel = DEFAULT_CKSEL;
+
+	if (get_of_u32(node, "fsl,cksel", &etsects->cksel))
+		etsects->cksel = DEFAULT_CKSEL;
 
 	if (get_of_u32(node, "fsl,tclk-period", &etsects->tclk_period) ||
 	    get_of_u32(node, "fsl,tmr-prsc", &etsects->tmr_prsc) ||
-- 
1.8.1.2

^ permalink raw reply related

* RE: [PATCH 1/2] net: Toeplitz library functions
From: David Laight @ 2013-09-24  8:32 UTC (permalink / raw)
  To: Tom Herbert, davem; +Cc: netdev, jesse.brandeburg
In-Reply-To: <alpine.DEB.2.02.1309231535030.23896@tomh.mtv.corp.google.com>

> +static inline unsigned int
> +toeplitz_hash(const unsigned char *bytes,
> +	      struct toeplitz *toeplitz, int n)
> +{
> +	int i;
> +	unsigned int result = 0;
> +
> +	for (i = 0; i < n; i++)
> +		result ^= toeplitz->key_cache[i][bytes[i]];
> +
> +        return result;
> +};

That is a horrid hash function to be calculating in software.

The code looks very much like a simple 32bit CRC.
It isn't entirely clears exactly where the 'key' gets included,
but I suspect it is just xored with the data bytes.

Using in it hardware is probably fine - the hardware can do
it cheaply (in dedicated logic) as the frame arrives.
The CRC polynomial probably collapses to a few XOR operations
when done byte by byte (the hdlc crc16 collapses to 3 levels
of xor).

IIRC jhash() works on 32bit quantities - so has far fewer
maths operations and well as not having all the random data
accesses (cache misses and displacing other parts of the
working set from the cache).

I also thought the hash was arranged so that tx and rx packets
for a single connection hash to the same value?

	David

^ permalink raw reply

* Re: [PATCH] stable_kernel_rules.txt: Exclude networking from stable rules
From: Christoph Hellwig @ 2013-09-24  8:48 UTC (permalink / raw)
  To: Joe Perches
  Cc: stephen, linux-doc, Greg Kroah-Hartman, LKML, xfs,
	Christoph Hellwig, Mikulas Patocka, Rob Landley, netdev,
	David Miller
In-Reply-To: <1379968445.3575.60.camel@joe-AO722>

On Mon, Sep 23, 2013 at 01:34:05PM -0700, Joe Perches wrote:
> Maybe adding a mechanism to MAINTAINERS would be better.
> Maybe a default B: (backport?) of stable@vger.kernel.org
> with a per-subsystem override?

Sounds fine to me.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

^ permalink raw reply

* Re: [PATCH] ipvs: improved SH fallback strategy
From: Alexander Frolkin @ 2013-09-24  9:32 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Julian Anastasov, lvs-devel, Wensong Zhang, Simon Horman, netdev,
	linux-kernel
In-Reply-To: <524099BA.5020303@cogentembedded.com>

Improve the SH fallback realserver selection strategy.
 
With sh and sh-fallback, if a realserver is down, this attempts to
distribute the traffic that would have gone to that server evenly
among the remaining servers.
 
Signed-off-by: Alexander Frolkin <avf@eldamar.org.uk>
--
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index 3588fae..0db7d01 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -120,22 +120,33 @@ static inline struct ip_vs_dest *
 ip_vs_sh_get_fallback(struct ip_vs_service *svc, struct ip_vs_sh_state *s,
 		      const union nf_inet_addr *addr, __be16 port)
 {
-	unsigned int offset;
-	unsigned int hash;
+	unsigned int offset, roffset;
+	unsigned int hash, ihash;
 	struct ip_vs_dest *dest;
 
-	for (offset = 0; offset < IP_VS_SH_TAB_SIZE; offset++) {
-		hash = ip_vs_sh_hashkey(svc->af, addr, port, offset);
-		dest = rcu_dereference(s->buckets[hash].dest);
-		if (!dest)
-			break;
-		if (is_unavailable(dest))
-			IP_VS_DBG_BUF(6, "SH: selected unavailable server "
-				      "%s:%d (offset %d)",
+	ihash = ip_vs_sh_hashkey(svc->af, addr, port, 0);
+	dest = rcu_dereference(s->buckets[ihash].dest);
+	if (!dest)
+		return NULL;
+	if (is_unavailable(dest)) {
+		IP_VS_DBG_BUF(6, "SH: selected unavailable server "
+		      "%s:%d, reselecting",
+		      IP_VS_DBG_ADDR(svc->af, &dest->addr),
+		      ntohs(dest->port));
+		for (offset = 0; offset < IP_VS_SH_TAB_SIZE; offset++) {
+			roffset = (offset + ihash) % IP_VS_SH_TAB_SIZE;
+			hash = ip_vs_sh_hashkey(svc->af, addr, port, roffset);
+			dest = rcu_dereference(s->buckets[hash].dest);
+			if (is_unavailable(dest))
+				IP_VS_DBG_BUF(6, "SH: selected unavailable "
+				      "server %s:%d (offset %d), reselecting",
 				      IP_VS_DBG_ADDR(svc->af, &dest->addr),
-				      ntohs(dest->port), offset);
-		else
-			return dest;
+				      ntohs(dest->port), roffset);
+			else
+				return dest;
+		}
+	} else {
+		return dest;
 	}
 
 	return NULL;

^ permalink raw reply related

* IP_FREEBIND with IPv6
From: David Madore @ 2013-09-24  9:24 UTC (permalink / raw)
  To: linux-netdev mailing-list

Dear list,

I have two questions regarding the IP_FREEBIND option in IPv6.

Consider the following script, which sets IP_FREEBIND and then tries
to bind to some arbitrary non-local IPv6 address:

### cut after ###
#! /usr/bin/env python
import socket
if not hasattr(socket, 'IP_FREEBIND'):
    socket.IP_FREEBIND = 15
s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
s.setsockopt(socket.SOL_IP, socket.IP_FREEBIND, 1)
s.bind(("fd42:dead:beef::1", 0))
print "success"
### cut before ###

* This fails (with EADDRNOTAVAIL) under the long-term 3.2 kernels (I
  tried with 3.2.50 and also the current Debian stable kernel,
  3.2.0-4-amd64).  On the other hand, it succeeds with a more recent
  kernel (I tried 3.10.10).  Is this to be considered a bug in the 3.2
  kernel?  I attempted to find which commit fixed this behavior,
  without success: can someone point out to the appropriate patch?
  Could this conceivably be proposed for inclusion in 3.2?

* If one sets net.ipv4.ip_nonlocal_bind to 1 then (according to ip(7))
  the script should work even without the s.setsockopt(socket.SOL_IP,
  socket.IP_FREEBIND, 1) line.  I have failed to make this work on any
  version of the kernel.  Again, is this a bug or a misunderstanding
  on my part of what net.ipv4.ip_nonlocal_bind should do?  (I am aware
  it says "ipv4" in the sysctl name, but since there is no
  corresponding net.ipv6.ip_nonlocal_bind, I don't see what else one
  is supposed to use.)

Best regards,

-- 
     David A. Madore
   ( http://www.madore.org/~david/ )

^ permalink raw reply

* [PATCH net-next 1/5] tipc: silence sparse warnings
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Andreas Bofjäll, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

Eliminate below sparse warnings:

net/tipc/link.c:1210:37: warning: cast removes address space of expression
net/tipc/link.c:1218:59: warning: incorrect type in argument 2 (different address spaces)
net/tipc/link.c:1218:59:    expected void const [noderef] <asn:1>*from
net/tipc/link.c:1218:59:    got unsigned char const [usertype] *[assigned] sect_crs
net/tipc/msg.c:96:61: warning: incorrect type in argument 3 (different address spaces)
net/tipc/msg.c:96:61:    expected void const *from
net/tipc/msg.c:96:61:    got void [noderef] <asn:1>*const iov_base
net/tipc/socket.c:341:49: warning: Using plain integer as NULL pointer
net/tipc/socket.c:1371:36: warning: Using plain integer as NULL pointer
net/tipc/socket.c:1694:57: warning: Using plain integer as NULL pointer

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Signed-off-by: Andreas Bofjäll <andreas.bofjall@ericsson.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c   |    5 +++--
 net/tipc/msg.c    |    4 ++--
 net/tipc/socket.c |    6 +++---
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 0cc3d90..40521ae 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1165,7 +1165,7 @@ static int link_send_sections_long(struct tipc_port *sender,
 	struct tipc_msg fragm_hdr;
 	struct sk_buff *buf, *buf_chain, *prev;
 	u32 fragm_crs, fragm_rest, hsz, sect_rest;
-	const unchar *sect_crs;
+	const unchar __user *sect_crs;
 	int curr_sect;
 	u32 fragm_no;
 	int res = 0;
@@ -1207,7 +1207,8 @@ again:
 
 		if (!sect_rest) {
 			sect_rest = msg_sect[++curr_sect].iov_len;
-			sect_crs = (const unchar *)msg_sect[curr_sect].iov_base;
+			sect_crs =
+			  (const unchar __user *)msg_sect[curr_sect].iov_base;
 		}
 
 		if (sect_rest < fragm_rest)
diff --git a/net/tipc/msg.c b/net/tipc/msg.c
index ced60e2..37cfb57 100644
--- a/net/tipc/msg.c
+++ b/net/tipc/msg.c
@@ -93,8 +93,8 @@ int tipc_msg_build(struct tipc_msg *hdr, struct iovec const *msg_sect,
 	skb_copy_to_linear_data(*buf, hdr, hsz);
 	for (res = 1, cnt = 0; res && (cnt < num_sect); cnt++) {
 		skb_copy_to_linear_data_offset(*buf, pos,
-					       msg_sect[cnt].iov_base,
-					       msg_sect[cnt].iov_len);
+			(const void __force *)msg_sect[cnt].iov_base,
+			msg_sect[cnt].iov_len);
 		pos += msg_sect[cnt].iov_len;
 	}
 	if (likely(res))
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 6cc7ddd..0ff921d 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -338,7 +338,7 @@ static int release(struct socket *sock)
 		buf = __skb_dequeue(&sk->sk_receive_queue);
 		if (buf == NULL)
 			break;
-		if (TIPC_SKB_CB(buf)->handle != 0)
+		if (TIPC_SKB_CB(buf)->handle != NULL)
 			kfree_skb(buf);
 		else {
 			if ((sock->state == SS_CONNECTING) ||
@@ -1368,7 +1368,7 @@ static u32 filter_rcv(struct sock *sk, struct sk_buff *buf)
 		return TIPC_ERR_OVERLOAD;
 
 	/* Enqueue message */
-	TIPC_SKB_CB(buf)->handle = 0;
+	TIPC_SKB_CB(buf)->handle = NULL;
 	__skb_queue_tail(&sk->sk_receive_queue, buf);
 	skb_set_owner_r(buf, sk);
 
@@ -1691,7 +1691,7 @@ restart:
 		/* Disconnect and send a 'FIN+' or 'FIN-' message to peer */
 		buf = __skb_dequeue(&sk->sk_receive_queue);
 		if (buf) {
-			if (TIPC_SKB_CB(buf)->handle != 0) {
+			if (TIPC_SKB_CB(buf)->handle != NULL) {
 				kfree_skb(buf);
 				goto restart;
 			}
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 0/5] tipc: some small patches
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy

Some small and relatively straightforward patches that fix a number of
minor issues. The patches are functionally unrelated to each other.

Ying Xue (5):
  tipc: silence sparse warnings
  tipc: make bearer and media naming consistent
  tipc: avoid unnecessary lookup for tipc bearer instance
  tipc: correct return value of recv_msg routine
  tipc: correct return value of link_cmd_set_value routine

 net/tipc/bearer.c    |   18 ++++---------
 net/tipc/bearer.h    |   10 ++++----
 net/tipc/eth_media.c |   68 +++++++++++++++++++++++++-------------------------
 net/tipc/ib_media.c  |   58 +++++++++++++++++++++---------------------
 net/tipc/link.c      |   33 ++++++++++++++++--------
 net/tipc/msg.c       |    4 +--
 net/tipc/socket.c    |    6 ++---
 7 files changed, 100 insertions(+), 97 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH net-next 2/5] tipc: make bearer and media naming consistent
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

TIPC 'bearer' exists as an abstract concept, while 'media'
is deemed a specific implementation of a bearer, such as Ethernet
or Infiniband media. When a component inside TIPC wants to control
a specific media, it only needs to access the generic bearer API
to achieve this. However, in the current media implementations,
the 'bearer' name is also extensively used in media specific
function and variable names.

This may create confusion, so we choose to replace the term 'bearer'
with 'media' in all function names, variable names, and prefixes
where this is what really is meant.

Note that this change is cosmetic only, and no runtime behaviour
changes are made here.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bearer.c    |    4 ++--
 net/tipc/bearer.h    |    8 ++++----
 net/tipc/eth_media.c |   56 +++++++++++++++++++++++++-------------------------
 net/tipc/ib_media.c  |   46 ++++++++++++++++++++---------------------
 4 files changed, 57 insertions(+), 57 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 609c30c..09faa55 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -387,7 +387,7 @@ restart:
 
 	b_ptr = &tipc_bearers[bearer_id];
 	strcpy(b_ptr->name, name);
-	res = m_ptr->enable_bearer(b_ptr);
+	res = m_ptr->enable_media(b_ptr);
 	if (res) {
 		pr_warn("Bearer <%s> rejected, enable failure (%d)\n",
 			name, -res);
@@ -465,7 +465,7 @@ static void bearer_disable(struct tipc_bearer *b_ptr)
 	pr_info("Disabling bearer <%s>\n", b_ptr->name);
 	spin_lock_bh(&b_ptr->lock);
 	b_ptr->blocked = 1;
-	b_ptr->media->disable_bearer(b_ptr);
+	b_ptr->media->disable_media(b_ptr);
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
 		tipc_link_delete(l_ptr);
 	}
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index 09c869a..f800e63 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -75,8 +75,8 @@ struct tipc_bearer;
 /**
  * struct tipc_media - TIPC media information available to internal users
  * @send_msg: routine which handles buffer transmission
- * @enable_bearer: routine which enables a bearer
- * @disable_bearer: routine which disables a bearer
+ * @enable_media: routine which enables a media
+ * @disable_media: routine which disables a media
  * @addr2str: routine which converts media address to string
  * @addr2msg: routine which converts media address to protocol message area
  * @msg2addr: routine which converts media address from protocol message area
@@ -91,8 +91,8 @@ struct tipc_media {
 	int (*send_msg)(struct sk_buff *buf,
 			struct tipc_bearer *b_ptr,
 			struct tipc_media_addr *dest);
-	int (*enable_bearer)(struct tipc_bearer *b_ptr);
-	void (*disable_bearer)(struct tipc_bearer *b_ptr);
+	int (*enable_media)(struct tipc_bearer *b_ptr);
+	void (*disable_media)(struct tipc_bearer *b_ptr);
 	int (*addr2str)(struct tipc_media_addr *a, char *str_buf, int str_size);
 	int (*addr2msg)(struct tipc_media_addr *a, char *msg_area);
 	int (*msg2addr)(const struct tipc_bearer *b_ptr,
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index 40ea40c..e048d49 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -2,7 +2,7 @@
  * net/tipc/eth_media.c: Ethernet bearer support for TIPC
  *
  * Copyright (c) 2001-2007, Ericsson AB
- * Copyright (c) 2005-2008, 2011, Wind River Systems
+ * Copyright (c) 2005-2008, 2011-2013, Wind River Systems
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -37,19 +37,19 @@
 #include "core.h"
 #include "bearer.h"
 
-#define MAX_ETH_BEARERS		MAX_BEARERS
+#define MAX_ETH_MEDIA		MAX_BEARERS
 
 #define ETH_ADDR_OFFSET	4	/* message header offset of MAC address */
 
 /**
- * struct eth_bearer - Ethernet bearer data structure
+ * struct eth_media - Ethernet bearer data structure
  * @bearer: ptr to associated "generic" bearer structure
  * @dev: ptr to associated Ethernet network device
  * @tipc_packet_type: used in binding TIPC to Ethernet driver
  * @setup: work item used when enabling bearer
  * @cleanup: work item used when disabling bearer
  */
-struct eth_bearer {
+struct eth_media {
 	struct tipc_bearer *bearer;
 	struct net_device *dev;
 	struct packet_type tipc_packet_type;
@@ -58,7 +58,7 @@ struct eth_bearer {
 };
 
 static struct tipc_media eth_media_info;
-static struct eth_bearer eth_bearers[MAX_ETH_BEARERS];
+static struct eth_media eth_media_array[MAX_ETH_MEDIA];
 static int eth_started;
 
 static int recv_notification(struct notifier_block *nb, unsigned long evt,
@@ -100,7 +100,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 	if (!clone)
 		return 0;
 
-	dev = ((struct eth_bearer *)(tb_ptr->usr_handle))->dev;
+	dev = ((struct eth_media *)(tb_ptr->usr_handle))->dev;
 	delta = dev->hard_header_len - skb_headroom(buf);
 
 	if ((delta > 0) &&
@@ -128,7 +128,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		    struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct eth_bearer *eb_ptr = (struct eth_bearer *)pt->af_packet_priv;
+	struct eth_media *eb_ptr = (struct eth_media *)pt->af_packet_priv;
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
@@ -147,24 +147,24 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 }
 
 /**
- * setup_bearer - setup association between Ethernet bearer and interface
+ * setup_media - setup association between Ethernet bearer and interface
  */
-static void setup_bearer(struct work_struct *work)
+static void setup_media(struct work_struct *work)
 {
-	struct eth_bearer *eb_ptr =
-		container_of(work, struct eth_bearer, setup);
+	struct eth_media *eb_ptr =
+		container_of(work, struct eth_media, setup);
 
 	dev_add_pack(&eb_ptr->tipc_packet_type);
 }
 
 /**
- * enable_bearer - attach TIPC bearer to an Ethernet interface
+ * enable_media - attach TIPC bearer to an Ethernet interface
  */
-static int enable_bearer(struct tipc_bearer *tb_ptr)
+static int enable_media(struct tipc_bearer *tb_ptr)
 {
 	struct net_device *dev;
-	struct eth_bearer *eb_ptr = &eth_bearers[0];
-	struct eth_bearer *stop = &eth_bearers[MAX_ETH_BEARERS];
+	struct eth_media *eb_ptr = &eth_media_array[0];
+	struct eth_media *stop = &eth_media_array[MAX_ETH_MEDIA];
 	char *driver_name = strchr((const char *)tb_ptr->name, ':') + 1;
 	int pending_dev = 0;
 
@@ -188,7 +188,7 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 	eb_ptr->tipc_packet_type.func = recv_msg;
 	eb_ptr->tipc_packet_type.af_packet_priv = eb_ptr;
 	INIT_LIST_HEAD(&(eb_ptr->tipc_packet_type.list));
-	INIT_WORK(&eb_ptr->setup, setup_bearer);
+	INIT_WORK(&eb_ptr->setup, setup_media);
 	schedule_work(&eb_ptr->setup);
 
 	/* Associate TIPC bearer with Ethernet bearer */
@@ -205,14 +205,14 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 }
 
 /**
- * cleanup_bearer - break association between Ethernet bearer and interface
+ * cleanup_media - break association between Ethernet bearer and interface
  *
  * This routine must be invoked from a work queue because it can sleep.
  */
-static void cleanup_bearer(struct work_struct *work)
+static void cleanup_media(struct work_struct *work)
 {
-	struct eth_bearer *eb_ptr =
-		container_of(work, struct eth_bearer, cleanup);
+	struct eth_media *eb_ptr =
+		container_of(work, struct eth_media, cleanup);
 
 	dev_remove_pack(&eb_ptr->tipc_packet_type);
 	dev_put(eb_ptr->dev);
@@ -220,18 +220,18 @@ static void cleanup_bearer(struct work_struct *work)
 }
 
 /**
- * disable_bearer - detach TIPC bearer from an Ethernet interface
+ * disable_media - detach TIPC bearer from an Ethernet interface
  *
  * Mark Ethernet bearer as inactive so that incoming buffers are thrown away,
  * then get worker thread to complete bearer cleanup.  (Can't do cleanup
  * here because cleanup code needs to sleep and caller holds spinlocks.)
  */
-static void disable_bearer(struct tipc_bearer *tb_ptr)
+static void disable_media(struct tipc_bearer *tb_ptr)
 {
-	struct eth_bearer *eb_ptr = (struct eth_bearer *)tb_ptr->usr_handle;
+	struct eth_media *eb_ptr = (struct eth_media *)tb_ptr->usr_handle;
 
 	eb_ptr->bearer = NULL;
-	INIT_WORK(&eb_ptr->cleanup, cleanup_bearer);
+	INIT_WORK(&eb_ptr->cleanup, cleanup_media);
 	schedule_work(&eb_ptr->cleanup);
 }
 
@@ -245,8 +245,8 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 			     void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	struct eth_bearer *eb_ptr = &eth_bearers[0];
-	struct eth_bearer *stop = &eth_bearers[MAX_ETH_BEARERS];
+	struct eth_media *eb_ptr = &eth_media_array[0];
+	struct eth_media *stop = &eth_media_array[MAX_ETH_MEDIA];
 
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
@@ -327,8 +327,8 @@ static int eth_msg2addr(const struct tipc_bearer *tb_ptr,
  */
 static struct tipc_media eth_media_info = {
 	.send_msg	= send_msg,
-	.enable_bearer	= enable_bearer,
-	.disable_bearer	= disable_bearer,
+	.enable_media	= enable_media,
+	.disable_media	= disable_media,
 	.addr2str	= eth_addr2str,
 	.addr2msg	= eth_addr2msg,
 	.msg2addr	= eth_msg2addr,
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 9934a32..5545145 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -42,17 +42,17 @@
 #include "core.h"
 #include "bearer.h"
 
-#define MAX_IB_BEARERS		MAX_BEARERS
+#define MAX_IB_MEDIA		MAX_BEARERS
 
 /**
- * struct ib_bearer - Infiniband bearer data structure
+ * struct ib_media - Infiniband media data structure
  * @bearer: ptr to associated "generic" bearer structure
  * @dev: ptr to associated Infiniband network device
  * @tipc_packet_type: used in binding TIPC to Infiniband driver
  * @cleanup: work item used when disabling bearer
  */
 
-struct ib_bearer {
+struct ib_media {
 	struct tipc_bearer *bearer;
 	struct net_device *dev;
 	struct packet_type tipc_packet_type;
@@ -61,7 +61,7 @@ struct ib_bearer {
 };
 
 static struct tipc_media ib_media_info;
-static struct ib_bearer ib_bearers[MAX_IB_BEARERS];
+static struct ib_media ib_media_array[MAX_IB_MEDIA];
 static int ib_started;
 
 /**
@@ -93,7 +93,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 	if (!clone)
 		return 0;
 
-	dev = ((struct ib_bearer *)(tb_ptr->usr_handle))->dev;
+	dev = ((struct ib_media *)(tb_ptr->usr_handle))->dev;
 	delta = dev->hard_header_len - skb_headroom(buf);
 
 	if ((delta > 0) &&
@@ -121,7 +121,7 @@ static int send_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr,
 static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 		    struct packet_type *pt, struct net_device *orig_dev)
 {
-	struct ib_bearer *ib_ptr = (struct ib_bearer *)pt->af_packet_priv;
+	struct ib_media *ib_ptr = (struct ib_media *)pt->af_packet_priv;
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
@@ -142,22 +142,22 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 /**
  * setup_bearer - setup association between InfiniBand bearer and interface
  */
-static void setup_bearer(struct work_struct *work)
+static void setup_media(struct work_struct *work)
 {
-	struct ib_bearer *ib_ptr =
-		container_of(work, struct ib_bearer, setup);
+	struct ib_media *ib_ptr =
+		container_of(work, struct ib_media, setup);
 
 	dev_add_pack(&ib_ptr->tipc_packet_type);
 }
 
 /**
- * enable_bearer - attach TIPC bearer to an InfiniBand interface
+ * enable_media - attach TIPC bearer to an InfiniBand interface
  */
-static int enable_bearer(struct tipc_bearer *tb_ptr)
+static int enable_media(struct tipc_bearer *tb_ptr)
 {
 	struct net_device *dev;
-	struct ib_bearer *ib_ptr = &ib_bearers[0];
-	struct ib_bearer *stop = &ib_bearers[MAX_IB_BEARERS];
+	struct ib_media *ib_ptr = &ib_media_array[0];
+	struct ib_media *stop = &ib_media_array[MAX_IB_MEDIA];
 	char *driver_name = strchr((const char *)tb_ptr->name, ':') + 1;
 	int pending_dev = 0;
 
@@ -181,7 +181,7 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
 	ib_ptr->tipc_packet_type.func = recv_msg;
 	ib_ptr->tipc_packet_type.af_packet_priv = ib_ptr;
 	INIT_LIST_HEAD(&(ib_ptr->tipc_packet_type.list));
-	INIT_WORK(&ib_ptr->setup, setup_bearer);
+	INIT_WORK(&ib_ptr->setup, setup_media);
 	schedule_work(&ib_ptr->setup);
 
 	/* Associate TIPC bearer with InfiniBand bearer */
@@ -204,8 +204,8 @@ static int enable_bearer(struct tipc_bearer *tb_ptr)
  */
 static void cleanup_bearer(struct work_struct *work)
 {
-	struct ib_bearer *ib_ptr =
-		container_of(work, struct ib_bearer, cleanup);
+	struct ib_media *ib_ptr =
+		container_of(work, struct ib_media, cleanup);
 
 	dev_remove_pack(&ib_ptr->tipc_packet_type);
 	dev_put(ib_ptr->dev);
@@ -213,15 +213,15 @@ static void cleanup_bearer(struct work_struct *work)
 }
 
 /**
- * disable_bearer - detach TIPC bearer from an InfiniBand interface
+ * disable_media - detach TIPC bearer from an InfiniBand interface
  *
  * Mark InfiniBand bearer as inactive so that incoming buffers are thrown away,
  * then get worker thread to complete bearer cleanup.  (Can't do cleanup
  * here because cleanup code needs to sleep and caller holds spinlocks.)
  */
-static void disable_bearer(struct tipc_bearer *tb_ptr)
+static void disable_media(struct tipc_bearer *tb_ptr)
 {
-	struct ib_bearer *ib_ptr = (struct ib_bearer *)tb_ptr->usr_handle;
+	struct ib_media *ib_ptr = (struct ib_media *)tb_ptr->usr_handle;
 
 	ib_ptr->bearer = NULL;
 	INIT_WORK(&ib_ptr->cleanup, cleanup_bearer);
@@ -238,8 +238,8 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 			     void *ptr)
 {
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	struct ib_bearer *ib_ptr = &ib_bearers[0];
-	struct ib_bearer *stop = &ib_bearers[MAX_IB_BEARERS];
+	struct ib_media *ib_ptr = &ib_media_array[0];
+	struct ib_media *stop = &ib_media_array[MAX_IB_MEDIA];
 
 	if (!net_eq(dev_net(dev), &init_net))
 		return NOTIFY_DONE;
@@ -323,8 +323,8 @@ static int ib_msg2addr(const struct tipc_bearer *tb_ptr,
  */
 static struct tipc_media ib_media_info = {
 	.send_msg	= send_msg,
-	.enable_bearer	= enable_bearer,
-	.disable_bearer	= disable_bearer,
+	.enable_media	= enable_media,
+	.disable_media	= disable_media,
 	.addr2str	= ib_addr2str,
 	.addr2msg	= ib_addr2msg,
 	.msg2addr	= ib_msg2addr,
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/5] tipc: avoid unnecessary lookup for tipc bearer instance
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

tipc_block_bearer() currently takes a bearer name (const char*)
as argument. This requires the function to make a lookup to find
the pointer to the corresponding bearer struct. In the current
code base this is not necessary, since the only two callers
(tipc_continue(),recv_notification()) already have validated
copies of this pointer, and hence can pass it directly in the
function call.

We change tipc_block_bearer() to directly take struct tipc_bearer*
as argument instead.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/bearer.c    |   14 +++-----------
 net/tipc/bearer.h    |    2 +-
 net/tipc/eth_media.c |    6 +++---
 net/tipc/ib_media.c  |    6 +++---
 4 files changed, 10 insertions(+), 18 deletions(-)

diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index 09faa55..3f9707a 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -420,23 +420,15 @@ exit:
 }
 
 /**
- * tipc_block_bearer - Block the bearer with the given name, and reset all its links
+ * tipc_block_bearer - Block the bearer, and reset all its links
  */
-int tipc_block_bearer(const char *name)
+int tipc_block_bearer(struct tipc_bearer *b_ptr)
 {
-	struct tipc_bearer *b_ptr = NULL;
 	struct tipc_link *l_ptr;
 	struct tipc_link *temp_l_ptr;
 
 	read_lock_bh(&tipc_net_lock);
-	b_ptr = tipc_bearer_find(name);
-	if (!b_ptr) {
-		pr_warn("Attempt to block unknown bearer <%s>\n", name);
-		read_unlock_bh(&tipc_net_lock);
-		return -EINVAL;
-	}
-
-	pr_info("Blocking bearer <%s>\n", name);
+	pr_info("Blocking bearer <%s>\n", b_ptr->name);
 	spin_lock_bh(&b_ptr->lock);
 	b_ptr->blocked = 1;
 	list_for_each_entry_safe(l_ptr, temp_l_ptr, &b_ptr->links, link_list) {
diff --git a/net/tipc/bearer.h b/net/tipc/bearer.h
index f800e63..e5e04be 100644
--- a/net/tipc/bearer.h
+++ b/net/tipc/bearer.h
@@ -163,7 +163,7 @@ int tipc_register_media(struct tipc_media *m_ptr);
 
 void tipc_recv_msg(struct sk_buff *buf, struct tipc_bearer *tb_ptr);
 
-int  tipc_block_bearer(const char *name);
+int  tipc_block_bearer(struct tipc_bearer *b_ptr);
 void tipc_continue(struct tipc_bearer *tb_ptr);
 
 int tipc_enable_bearer(const char *bearer_name, u32 disc_domain, u32 priority);
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index e048d49..c36c938 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -265,17 +265,17 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (netif_carrier_ok(dev))
 			tipc_continue(eb_ptr->bearer);
 		else
-			tipc_block_bearer(eb_ptr->bearer->name);
+			tipc_block_bearer(eb_ptr->bearer);
 		break;
 	case NETDEV_UP:
 		tipc_continue(eb_ptr->bearer);
 		break;
 	case NETDEV_DOWN:
-		tipc_block_bearer(eb_ptr->bearer->name);
+		tipc_block_bearer(eb_ptr->bearer);
 		break;
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
-		tipc_block_bearer(eb_ptr->bearer->name);
+		tipc_block_bearer(eb_ptr->bearer);
 		tipc_continue(eb_ptr->bearer);
 		break;
 	case NETDEV_UNREGISTER:
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 5545145..20b1aa4 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -258,17 +258,17 @@ static int recv_notification(struct notifier_block *nb, unsigned long evt,
 		if (netif_carrier_ok(dev))
 			tipc_continue(ib_ptr->bearer);
 		else
-			tipc_block_bearer(ib_ptr->bearer->name);
+			tipc_block_bearer(ib_ptr->bearer);
 		break;
 	case NETDEV_UP:
 		tipc_continue(ib_ptr->bearer);
 		break;
 	case NETDEV_DOWN:
-		tipc_block_bearer(ib_ptr->bearer->name);
+		tipc_block_bearer(ib_ptr->bearer);
 		break;
 	case NETDEV_CHANGEMTU:
 	case NETDEV_CHANGEADDR:
-		tipc_block_bearer(ib_ptr->bearer->name);
+		tipc_block_bearer(ib_ptr->bearer);
 		tipc_continue(ib_ptr->bearer);
 		break;
 	case NETDEV_UNREGISTER:
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 4/5] tipc: correct return value of recv_msg routine
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

Currently, rcv_msg() always returns zero on a packet delivery upcall
from net_device.

To make its behavior more compliant with the way this API should be
used, we change this to let it return NET_RX_SUCCESS (which is zero
anyway) when it is able to handle the packet, and NET_RX_DROP otherwise.
The latter does not imply any functional change, it only enables the
driver to keep more accurate statistics about the fate of delivered
packets.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/eth_media.c |    6 +++---
 net/tipc/ib_media.c  |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index c36c938..f80d59f 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -132,18 +132,18 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
-		return 0;
+		return NET_RX_DROP;
 	}
 
 	if (likely(eb_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, eb_ptr->bearer);
-			return 0;
+			return NET_RX_SUCCESS;
 		}
 	}
 	kfree_skb(buf);
-	return 0;
+	return NET_RX_DROP;
 }
 
 /**
diff --git a/net/tipc/ib_media.c b/net/tipc/ib_media.c
index 20b1aa4..c139892 100644
--- a/net/tipc/ib_media.c
+++ b/net/tipc/ib_media.c
@@ -125,18 +125,18 @@ static int recv_msg(struct sk_buff *buf, struct net_device *dev,
 
 	if (!net_eq(dev_net(dev), &init_net)) {
 		kfree_skb(buf);
-		return 0;
+		return NET_RX_DROP;
 	}
 
 	if (likely(ib_ptr->bearer)) {
 		if (likely(buf->pkt_type <= PACKET_BROADCAST)) {
 			buf->next = NULL;
 			tipc_recv_msg(buf, ib_ptr->bearer);
-			return 0;
+			return NET_RX_SUCCESS;
 		}
 	}
 	kfree_skb(buf);
-	return 0;
+	return NET_RX_DROP;
 }
 
 /**
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 5/5] tipc: correct return value of link_cmd_set_value routine
From: Jon Maloy @ 2013-09-24  9:27 UTC (permalink / raw)
  To: davem
  Cc: netdev, Paul Gortmaker, erik.hugne, ying.xue, maloy,
	tipc-discussion, Jon Maloy
In-Reply-To: <1380014868-2797-1-git-send-email-jon.maloy@ericsson.com>

From: Ying Xue <ying.xue@windriver.com>

link_cmd_set_value() takes commands for link, bearer and media related
configuration. Genereally the function returns 0 when a command is
recognized, and -EINVAL when it is not. However, in the switch for link
related commands it returns 0 even when the command is unrecognized. This
will sometimes make it look as if a failed configuration command has been
successful, but has otherwise no negative effects.

We remove this anomaly by returning -EINVAL even for link commands. We also
rework all three switches to make them  conforming to common kernel coding
style.

Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/link.c |   28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/net/tipc/link.c b/net/tipc/link.c
index 40521ae..8bbe4ca 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -2647,6 +2647,7 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 	struct tipc_link *l_ptr;
 	struct tipc_bearer *b_ptr;
 	struct tipc_media *m_ptr;
+	int res = 0;
 
 	l_ptr = link_find_link(name, &node);
 	if (l_ptr) {
@@ -2669,9 +2670,12 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 		case TIPC_CMD_SET_LINK_WINDOW:
 			tipc_link_set_queue_limits(l_ptr, new_value);
 			break;
+		default:
+			res = -EINVAL;
+			break;
 		}
 		tipc_node_unlock(node);
-		return 0;
+		return res;
 	}
 
 	b_ptr = tipc_bearer_find(name);
@@ -2679,15 +2683,18 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 		switch (cmd) {
 		case TIPC_CMD_SET_LINK_TOL:
 			b_ptr->tolerance = new_value;
-			return 0;
+			break;
 		case TIPC_CMD_SET_LINK_PRI:
 			b_ptr->priority = new_value;
-			return 0;
+			break;
 		case TIPC_CMD_SET_LINK_WINDOW:
 			b_ptr->window = new_value;
-			return 0;
+			break;
+		default:
+			res = -EINVAL;
+			break;
 		}
-		return -EINVAL;
+		return res;
 	}
 
 	m_ptr = tipc_media_find(name);
@@ -2696,15 +2703,18 @@ static int link_cmd_set_value(const char *name, u32 new_value, u16 cmd)
 	switch (cmd) {
 	case TIPC_CMD_SET_LINK_TOL:
 		m_ptr->tolerance = new_value;
-		return 0;
+		break;
 	case TIPC_CMD_SET_LINK_PRI:
 		m_ptr->priority = new_value;
-		return 0;
+		break;
 	case TIPC_CMD_SET_LINK_WINDOW:
 		m_ptr->window = new_value;
-		return 0;
+		break;
+	default:
+		res = -EINVAL;
+		break;
 	}
-	return -EINVAL;
+	return res;
 }
 
 struct sk_buff *tipc_link_cmd_config(const void *req_tlv_area, int req_tlv_space,
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH net-next] xen-netback: improve ring effeciency for guest RX
From: Ian Campbell @ 2013-09-24  9:41 UTC (permalink / raw)
  To: Wei Liu; +Cc: netdev, xen-devel, Xi Xiong, Matt Wilson, Annie Li, David Vrabel
In-Reply-To: <1379873024-32132-1-git-send-email-wei.liu2@citrix.com>

On Sun, 2013-09-22 at 19:03 +0100, Wei Liu wrote:
> There was a bug that netback routines netbk/xenvif_skb_count_slots and
> netbk/xenvif_gop_frag_copy disagreed with each other, which caused
> netback to push wrong number of responses to netfront, which caused
> netfront to eventually crash. The bug was fixed in 6e43fc04a
> ("xen-netback: count number required slots for an skb more carefully").
> 
> Commit 6e43fc04a focused on backport-ability. The drawback with the
> existing packing scheme is that the ring is not used effeciently, as
> stated in 6e43fc04a.
> 
> skb->data like:
>     |        1111|222222222222|3333        |
> 
> is arranged as:
>     |1111        |222222222222|3333        |
> 
> If we can do this:
>     |111122222222|22223333    |
> That would save one ring slot, which improves ring effeciency.
> 
> This patch effectively reverts 6e43fc04a. That patch made count_slots
> agree with gop_frag_copy, while this patch goes the other way around --
> make gop_frag_copy agree with count_slots. The end result is that they
> still agree with each other, and the ring is now arranged like:
>     |111122222222|22223333    |
> 
> The patch that improves packing was first posted by Xi Xong and Matt
> Wilson. I only rebase it on top of net-next and rewrite commit message,
> so I retain all their SoBs. For more infomation about the original bug
> please refer to email listed below and commit message of 6e43fc04a.
> 
> Original patch:
> http://lists.xen.org/archives/html/xen-devel/2013-07/msg00760.html
> 
> Signed-off-by: Xi Xiong <xixiong@amazon.com>
> Reviewed-by: Matt Wilson <msw@amazon.com>
> [ msw: minor code cleanups, rewrote commit message, adjusted code
>   to count RX slots instead of meta structures ]
> Signed-off-by: Matt Wilson <msw@amazon.com>
> Cc: Annie Li <annie.li@oracle.com>
> Cc: Wei Liu <wei.liu2@citrix.com>

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

> [ liuw: rebased on top of net-next tree, rewrote commit message, coding
>   style cleanup. ]
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
>  drivers/net/xen-netback/netback.c |  144 ++++++++++++++++---------------------
>  1 file changed, 61 insertions(+), 83 deletions(-)
> 
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index f3e591c..d0b0feb 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -47,6 +47,14 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/page.h>
>  
> +/* SKB control block overlay is used to store useful information when
> + * doing guest RX.
> + */
> +struct skb_cb_overlay {
> +	int meta_slots_used;
> +	int peek_slots_count;
> +};
> +
>  /* Provide an option to disable split event channels at load time as
>   * event channels are limited resource. Split event channels are
>   * enabled by default.
> @@ -212,49 +220,6 @@ static bool start_new_rx_buffer(int offset, unsigned long size, int head)
>  	return false;
>  }
>  
> -struct xenvif_count_slot_state {
> -	unsigned long copy_off;
> -	bool head;
> -};
> -
> -unsigned int xenvif_count_frag_slots(struct xenvif *vif,
> -				     unsigned long offset, unsigned long size,
> -				     struct xenvif_count_slot_state *state)
> -{
> -	unsigned count = 0;
> -
> -	offset &= ~PAGE_MASK;
> -
> -	while (size > 0) {
> -		unsigned long bytes;
> -
> -		bytes = PAGE_SIZE - offset;
> -
> -		if (bytes > size)
> -			bytes = size;
> -
> -		if (start_new_rx_buffer(state->copy_off, bytes, state->head)) {
> -			count++;
> -			state->copy_off = 0;
> -		}
> -
> -		if (state->copy_off + bytes > MAX_BUFFER_OFFSET)
> -			bytes = MAX_BUFFER_OFFSET - state->copy_off;
> -
> -		state->copy_off += bytes;
> -
> -		offset += bytes;
> -		size -= bytes;
> -
> -		if (offset == PAGE_SIZE)
> -			offset = 0;
> -
> -		state->head = false;
> -	}
> -
> -	return count;
> -}
> -
>  /*
>   * Figure out how many ring slots we're going to need to send @skb to
>   * the guest. This function is essentially a dry run of
> @@ -262,40 +227,53 @@ unsigned int xenvif_count_frag_slots(struct xenvif *vif,
>   */
>  unsigned int xenvif_count_skb_slots(struct xenvif *vif, struct sk_buff *skb)
>  {
> -	struct xenvif_count_slot_state state;
>  	unsigned int count;
> -	unsigned char *data;
> -	unsigned i;
> +	int i, copy_off;
> +	struct skb_cb_overlay *sco;
>  
> -	state.head = true;
> -	state.copy_off = 0;
> +	count = DIV_ROUND_UP(skb_headlen(skb), PAGE_SIZE);
>  
> -	/* Slot for the first (partial) page of data. */
> -	count = 1;
> +	copy_off = skb_headlen(skb) % PAGE_SIZE;
>  
> -	/* Need a slot for the GSO prefix for GSO extra data? */
>  	if (skb_shinfo(skb)->gso_size)
>  		count++;
>  
> -	data = skb->data;
> -	while (data < skb_tail_pointer(skb)) {
> -		unsigned long offset = offset_in_page(data);
> -		unsigned long size = PAGE_SIZE - offset;
> +	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> +		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> +		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> +		unsigned long bytes;
>  
> -		if (data + size > skb_tail_pointer(skb))
> -			size = skb_tail_pointer(skb) - data;
> +		offset &= ~PAGE_MASK;
>  
> -		count += xenvif_count_frag_slots(vif, offset, size, &state);
> +		while (size > 0) {
> +			BUG_ON(offset >= PAGE_SIZE);
> +			BUG_ON(copy_off > MAX_BUFFER_OFFSET);
>  
> -		data += size;
> -	}
> +			bytes = PAGE_SIZE - offset;
>  
> -	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
> -		unsigned long size = skb_frag_size(&skb_shinfo(skb)->frags[i]);
> -		unsigned long offset = skb_shinfo(skb)->frags[i].page_offset;
> +			if (bytes > size)
> +				bytes = size;
> +
> +			if (start_new_rx_buffer(copy_off, bytes, 0)) {
> +				count++;
> +				copy_off = 0;
> +			}
>  
> -		count += xenvif_count_frag_slots(vif, offset, size, &state);
> +			if (copy_off + bytes > MAX_BUFFER_OFFSET)
> +				bytes = MAX_BUFFER_OFFSET - copy_off;
> +
> +			copy_off += bytes;
> +
> +			offset += bytes;
> +			size -= bytes;
> +
> +			if (offset == PAGE_SIZE)
> +				offset = 0;
> +		}
>  	}
> +
> +	sco = (struct skb_cb_overlay *)skb->cb;
> +	sco->peek_slots_count = count;
>  	return count;
>  }
>  
> @@ -327,14 +305,11 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif *vif,
>  	return meta;
>  }
>  
> -/*
> - * Set up the grant operations for this fragment. If it's a flipping
> - * interface, we also set up the unmap request from here.
> - */
> +/* Set up the grant operations for this fragment. */
>  static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  				 struct netrx_pending_operations *npo,
>  				 struct page *page, unsigned long size,
> -				 unsigned long offset, int *head)
> +				 unsigned long offset, int head, int *first)
>  {
>  	struct gnttab_copy *copy_gop;
>  	struct xenvif_rx_meta *meta;
> @@ -358,12 +333,12 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  		if (bytes > size)
>  			bytes = size;
>  
> -		if (start_new_rx_buffer(npo->copy_off, bytes, *head)) {
> +		if (start_new_rx_buffer(npo->copy_off, bytes, head)) {
>  			/*
>  			 * Netfront requires there to be some data in the head
>  			 * buffer.
>  			 */
> -			BUG_ON(*head);
> +			BUG_ON(*first);
>  
>  			meta = get_next_rx_buffer(vif, npo);
>  		}
> @@ -397,10 +372,10 @@ static void xenvif_gop_frag_copy(struct xenvif *vif, struct sk_buff *skb,
>  		}
>  
>  		/* Leave a gap for the GSO descriptor. */
> -		if (*head && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
> +		if (*first && skb_shinfo(skb)->gso_size && !vif->gso_prefix)
>  			vif->rx.req_cons++;
>  
> -		*head = 0; /* There must be something in this buffer now. */
> +		*first = 0; /* There must be something in this buffer now. */
>  
>  	}
>  }
> @@ -426,7 +401,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  	struct xen_netif_rx_request *req;
>  	struct xenvif_rx_meta *meta;
>  	unsigned char *data;
> -	int head = 1;
> +	int first = 1;
>  	int old_meta_prod;
>  
>  	old_meta_prod = npo->meta_prod;
> @@ -462,7 +437,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  			len = skb_tail_pointer(skb) - data;
>  
>  		xenvif_gop_frag_copy(vif, skb, npo,
> -				     virt_to_page(data), len, offset, &head);
> +				     virt_to_page(data), len, offset, 1, &first);
>  		data += len;
>  	}
>  
> @@ -471,7 +446,7 @@ static int xenvif_gop_skb(struct sk_buff *skb,
>  				     skb_frag_page(&skb_shinfo(skb)->frags[i]),
>  				     skb_frag_size(&skb_shinfo(skb)->frags[i]),
>  				     skb_shinfo(skb)->frags[i].page_offset,
> -				     &head);
> +				     0, &first);
>  	}
>  
>  	return npo->meta_prod - old_meta_prod;
> @@ -529,10 +504,6 @@ static void xenvif_add_frag_responses(struct xenvif *vif, int status,
>  	}
>  }
>  
> -struct skb_cb_overlay {
> -	int meta_slots_used;
> -};
> -
>  static void xenvif_kick_thread(struct xenvif *vif)
>  {
>  	wake_up(&vif->wq);
> @@ -563,19 +534,26 @@ void xenvif_rx_action(struct xenvif *vif)
>  	count = 0;
>  
>  	while ((skb = skb_dequeue(&vif->rx_queue)) != NULL) {
> +		RING_IDX old_rx_req_cons;
> +
>  		vif = netdev_priv(skb->dev);
>  		nr_frags = skb_shinfo(skb)->nr_frags;
>  
> +		old_rx_req_cons = vif->rx.req_cons;
>  		sco = (struct skb_cb_overlay *)skb->cb;
>  		sco->meta_slots_used = xenvif_gop_skb(skb, &npo);
>  
> -		count += nr_frags + 1;
> +		count += vif->rx.req_cons - old_rx_req_cons;
>  
>  		__skb_queue_tail(&rxq, skb);
>  
> +		skb = skb_peek(&vif->rx_queue);
> +		if (skb == NULL)
> +			break;
> +		sco = (struct skb_cb_overlay *)skb->cb;
> +
>  		/* Filled the batch queue? */
> -		/* XXX FIXME: RX path dependent on MAX_SKB_FRAGS */
> -		if (count + MAX_SKB_FRAGS >= XEN_NETIF_RX_RING_SIZE)
> +		if (count + sco->peek_slots_count >= XEN_NETIF_RX_RING_SIZE)
>  			break;
>  	}
>  

^ permalink raw reply

* [net 1/6] igb: Fix ethtool loopback test for 82580 copper
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Todd Fujinaka, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Todd Fujinaka <todd.fujinaka@intel.com>

Add back 82580 loopback tests to ethtool.

Signed-off-by: Todd Fujinaka <todd.fujinaka@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 48cbc83..86d5142 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1607,6 +1607,9 @@ static int igb_integrated_phy_loopback(struct igb_adapter *adapter)
 			igb_write_phy_reg(hw, I347AT4_PAGE_SELECT, 0);
 			igb_write_phy_reg(hw, PHY_CONTROL, 0x4140);
 		}
+	} else if (hw->phy.type == e1000_phy_82580) {
+		/* enable MII loopback */
+		igb_write_phy_reg(hw, I82580_PHY_LBK_CTRL, 0x8041);
 	}
 
 	/* add small delay to avoid loopback test failure */
-- 
1.8.3.1

^ permalink raw reply related

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

This series contains updates to igb and i40e.

Todd provides a fix for 82580 devices in igb, where the ethtool
loopback test was missing 82580 copper devices.

Jesse provides five fixes/cleanups to i40e based on feedback from
Joe Perches and the community.

The following are changes since commit 9fe34f5d920b183ec063550e0f4ec854aa373316:
  mrp: add periodictimer to allow retries when packets get lost
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net master

Todd Fujinaka (1):
  igb: Fix ethtool loopback test for 82580 copper

Jesse Brandeburg (5):
  i40e: use common failure flow
  i40e: small clean ups from review
  i40e: convert ret to aq_ret
  i40e: better return values
  i40e: clean up coccicheck reported errors

 drivers/net/ethernet/intel/i40e/i40e_adminq.c |   7 +-
 drivers/net/ethernet/intel/i40e/i40e_common.c |   2 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c   | 162 +++++++++++++-------------
 drivers/net/ethernet/intel/igb/igb_ethtool.c  |   3 +
 4 files changed, 89 insertions(+), 85 deletions(-)

-- 
1.8.3.1

^ permalink raw reply

* [net 3/6] i40e: small clean ups from review
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches clean up a loop flow.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 67f8fd5..865bc6b 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -174,8 +174,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 			 u16 needed, u16 id)
 {
 	int ret = -ENOMEM;
-	int i = 0;
-	int j = 0;
+	int i, j;
 
 	if (!pile || needed == 0 || id >= I40E_PILE_VALID_BIT) {
 		dev_info(&pf->pdev->dev,
@@ -186,7 +185,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 
 	/* start the linear search with an imperfect hint */
 	i = pile->search_hint;
-	while (i < pile->num_entries && ret < 0) {
+	while (i < pile->num_entries) {
 		/* skip already allocated entries */
 		if (pile->list[i] & I40E_PILE_VALID_BIT) {
 			i++;
@@ -205,6 +204,7 @@ static int i40e_get_lump(struct i40e_pf *pf, struct i40e_lump_tracking *pile,
 				pile->list[i+j] = id | I40E_PILE_VALID_BIT;
 			ret = i;
 			pile->search_hint = i + j;
+			break;
 		} else {
 			/* not enough, so skip over it and continue looking */
 			i += j;
-- 
1.8.3.1

^ permalink raw reply related

* [net 2/6] i40e: use common failure flow
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches, we should be using
foo = alloc(...)
if (!foo)
	return -ENOMEM;

return 0;

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 601d482..67f8fd5 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -101,10 +101,10 @@ int i40e_allocate_dma_mem_d(struct i40e_hw *hw, struct i40e_dma_mem *mem,
 	mem->size = ALIGN(size, alignment);
 	mem->va = dma_zalloc_coherent(&pf->pdev->dev, mem->size,
 				      &mem->pa, GFP_KERNEL);
-	if (mem->va)
-		return 0;
+	if (!mem->va)
+		return -ENOMEM;
 
-	return -ENOMEM;
+	return 0;
 }
 
 /**
@@ -136,10 +136,10 @@ int i40e_allocate_virt_mem_d(struct i40e_hw *hw, struct i40e_virt_mem *mem,
 	mem->size = size;
 	mem->va = kzalloc(size, GFP_KERNEL);
 
-	if (mem->va)
-		return 0;
+	if (!mem->va)
+		return -ENOMEM;
 
-	return -ENOMEM;
+	return 0;
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [net 4/6] i40e: convert ret to aq_ret
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

When calling admin queue functions the driver should use aq_ret
variable to help make clear that the return value is not a regular
return variable.

This allows for clean up of the return types that were previously
converted to int.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 107 ++++++++++++++--------------
 1 file changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 865bc6b..60c7152 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1388,7 +1388,7 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 	bool add_happened = false;
 	int filter_list_len = 0;
 	u32 changed_flags = 0;
-	i40e_status ret = 0;
+	i40e_status aq_ret = 0;
 	struct i40e_pf *pf;
 	int num_add = 0;
 	int num_del = 0;
@@ -1449,28 +1449,28 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 
 			/* flush a full buffer */
 			if (num_del == filter_list_len) {
-				ret = i40e_aq_remove_macvlan(&pf->hw,
+				aq_ret = i40e_aq_remove_macvlan(&pf->hw,
 					    vsi->seid, del_list, num_del,
 					    NULL);
 				num_del = 0;
 				memset(del_list, 0, sizeof(*del_list));
 
-				if (ret)
+				if (aq_ret)
 					dev_info(&pf->pdev->dev,
 						 "ignoring delete macvlan error, err %d, aq_err %d while flushing a full buffer\n",
-						 ret,
+						 aq_ret,
 						 pf->hw.aq.asq_last_status);
 			}
 		}
 		if (num_del) {
-			ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
+			aq_ret = i40e_aq_remove_macvlan(&pf->hw, vsi->seid,
 						     del_list, num_del, NULL);
 			num_del = 0;
 
-			if (ret)
+			if (aq_ret)
 				dev_info(&pf->pdev->dev,
 					 "ignoring delete macvlan error, err %d, aq_err %d\n",
-					 ret, pf->hw.aq.asq_last_status);
+					 aq_ret, pf->hw.aq.asq_last_status);
 		}
 
 		kfree(del_list);
@@ -1515,32 +1515,30 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 
 			/* flush a full buffer */
 			if (num_add == filter_list_len) {
-				ret = i40e_aq_add_macvlan(&pf->hw,
-							  vsi->seid,
-							  add_list,
-							  num_add,
-							  NULL);
+				aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+							     add_list, num_add,
+							     NULL);
 				num_add = 0;
 
-				if (ret)
+				if (aq_ret)
 					break;
 				memset(add_list, 0, sizeof(*add_list));
 			}
 		}
 		if (num_add) {
-			ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
-						  add_list, num_add, NULL);
+			aq_ret = i40e_aq_add_macvlan(&pf->hw, vsi->seid,
+						     add_list, num_add, NULL);
 			num_add = 0;
 		}
 		kfree(add_list);
 		add_list = NULL;
 
-		if (add_happened && (!ret)) {
+		if (add_happened && (!aq_ret)) {
 			/* do nothing */;
-		} else if (add_happened && (ret)) {
+		} else if (add_happened && (aq_ret)) {
 			dev_info(&pf->pdev->dev,
 				 "add filter failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 			if ((pf->hw.aq.asq_last_status == I40E_AQ_RC_ENOSPC) &&
 			    !test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
 				      &vsi->state)) {
@@ -1556,28 +1554,27 @@ int i40e_sync_vsi_filters(struct i40e_vsi *vsi)
 	if (changed_flags & IFF_ALLMULTI) {
 		bool cur_multipromisc;
 		cur_multipromisc = !!(vsi->current_netdev_flags & IFF_ALLMULTI);
-		ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
-							    vsi->seid,
-							    cur_multipromisc,
-							    NULL);
-		if (ret)
+		aq_ret = i40e_aq_set_vsi_multicast_promiscuous(&vsi->back->hw,
+							       vsi->seid,
+							       cur_multipromisc,
+							       NULL);
+		if (aq_ret)
 			dev_info(&pf->pdev->dev,
 				 "set multi promisc failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 	}
 	if ((changed_flags & IFF_PROMISC) || promisc_forced_on) {
 		bool cur_promisc;
 		cur_promisc = (!!(vsi->current_netdev_flags & IFF_PROMISC) ||
 			       test_bit(__I40E_FILTER_OVERFLOW_PROMISC,
 					&vsi->state));
-		ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
-							  vsi->seid,
-							  cur_promisc,
-							  NULL);
-		if (ret)
+		aq_ret = i40e_aq_set_vsi_unicast_promiscuous(&vsi->back->hw,
+							     vsi->seid,
+							     cur_promisc, NULL);
+		if (aq_ret)
 			dev_info(&pf->pdev->dev,
 				 "set uni promisc failed, err %d, aq_err %d\n",
-				 ret, pf->hw.aq.asq_last_status);
+				 aq_ret, pf->hw.aq.asq_last_status);
 	}
 
 	clear_bit(__I40E_CONFIG_BUSY, &vsi->state);
@@ -1936,10 +1933,10 @@ static void i40e_restore_vlan(struct i40e_vsi *vsi)
  * @vsi: the vsi being adjusted
  * @vid: the vlan id to set as a PVID
  **/
-i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
+int i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
 {
 	struct i40e_vsi_context ctxt;
-	i40e_status ret;
+	i40e_status aq_ret;
 
 	vsi->info.valid_sections = cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
 	vsi->info.pvid = cpu_to_le16(vid);
@@ -1948,14 +1945,15 @@ i40e_status i40e_vsi_add_pvid(struct i40e_vsi *vsi, u16 vid)
 
 	ctxt.seid = vsi->seid;
 	memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info));
-	ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_update_vsi_params(&vsi->back->hw, &ctxt, NULL);
+	if (aq_ret) {
 		dev_info(&vsi->back->pdev->dev,
 			 "%s: update vsi failed, aq_err=%d\n",
 			 __func__, vsi->back->hw.aq.asq_last_status);
+		return -ENOENT;
 	}
 
-	return ret;
+	return 0;
 }
 
 /**
@@ -3451,28 +3449,27 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 	struct i40e_aqc_query_vsi_bw_config_resp bw_config = {0};
 	struct i40e_pf *pf = vsi->back;
 	struct i40e_hw *hw = &pf->hw;
+	i40e_status aq_ret;
 	u32 tc_bw_max;
-	int ret;
 	int i;
 
 	/* Get the VSI level BW configuration */
-	ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_query_vsi_bw_config(hw, vsi->seid, &bw_config, NULL);
+	if (aq_ret) {
 		dev_info(&pf->pdev->dev,
 			 "couldn't get pf vsi bw config, err %d, aq_err %d\n",
-			 ret, pf->hw.aq.asq_last_status);
-		return ret;
+			 aq_ret, pf->hw.aq.asq_last_status);
+		return -EINVAL;
 	}
 
 	/* Get the VSI level BW configuration per TC */
-	ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid,
-					       &bw_ets_config,
-					       NULL);
-	if (ret) {
+	aq_ret = i40e_aq_query_vsi_ets_sla_config(hw, vsi->seid, &bw_ets_config,
+					          NULL);
+	if (aq_ret) {
 		dev_info(&pf->pdev->dev,
 			 "couldn't get pf vsi ets bw config, err %d, aq_err %d\n",
-			 ret, pf->hw.aq.asq_last_status);
-		return ret;
+			 aq_ret, pf->hw.aq.asq_last_status);
+		return -EINVAL;
 	}
 
 	if (bw_config.tc_valid_bits != bw_ets_config.tc_valid_bits) {
@@ -3494,7 +3491,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 		/* 3 bits out of 4 for each TC */
 		vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
 	}
-	return ret;
+	return 0;
 }
 
 /**
@@ -3505,30 +3502,30 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
  *
  * Returns 0 on success, negative value on failure
  **/
-static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi,
-				       u8 enabled_tc,
+static int i40e_vsi_configure_bw_alloc(struct i40e_vsi *vsi, u8 enabled_tc,
 				       u8 *bw_share)
 {
 	struct i40e_aqc_configure_vsi_tc_bw_data bw_data;
-	int i, ret = 0;
+	i40e_status aq_ret;
+	int i;
 
 	bw_data.tc_valid_bits = enabled_tc;
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
 		bw_data.tc_bw_credits[i] = bw_share[i];
 
-	ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid,
-				       &bw_data, NULL);
-	if (ret) {
+	aq_ret = i40e_aq_config_vsi_tc_bw(&vsi->back->hw, vsi->seid, &bw_data,
+					  NULL);
+	if (aq_ret) {
 		dev_info(&vsi->back->pdev->dev,
 			 "%s: AQ command Config VSI BW allocation per TC failed = %d\n",
 			 __func__, vsi->back->hw.aq.asq_last_status);
-		return ret;
+		return -EINVAL;
 	}
 
 	for (i = 0; i < I40E_MAX_TRAFFIC_CLASS; i++)
 		vsi->info.qs_handle[i] = bw_data.qs_handles[i];
 
-	return ret;
+	return 0;
 }
 
 /**
-- 
1.8.3.1

^ permalink raw reply related

* [net 6/6] i40e: clean up coccicheck reported errors
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

coccicheck shows:

drivers/net/ethernet/intel/i40e/i40e_adminq.c:704:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:763:1-7: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_adminq.c:810:2-8: Replace memcpy
with struct assignment
drivers/net/ethernet/intel/i40e/i40e_common.c:510:2-8: Replace memcpy
with struct assignment

Fix each of them with a *a = *b;

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_adminq.c | 7 +++----
 drivers/net/ethernet/intel/i40e/i40e_common.c | 2 +-
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_adminq.c b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
index 0c524fa..cfef7fc 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_adminq.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_adminq.c
@@ -701,8 +701,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 
 	details = I40E_ADMINQ_DETAILS(hw->aq.asq, hw->aq.asq.next_to_use);
 	if (cmd_details) {
-		memcpy(details, cmd_details,
-		       sizeof(struct i40e_asq_cmd_details));
+		*details = *cmd_details;
 
 		/* If the cmd_details are defined copy the cookie.  The
 		 * cpu_to_le32 is not needed here because the data is ignored
@@ -760,7 +759,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 	desc_on_ring = I40E_ADMINQ_DESC(hw->aq.asq, hw->aq.asq.next_to_use);
 
 	/* if the desc is available copy the temp desc to the right place */
-	memcpy(desc_on_ring, desc, sizeof(struct i40e_aq_desc));
+	*desc_on_ring = *desc;
 
 	/* if buff is not NULL assume indirect command */
 	if (buff != NULL) {
@@ -807,7 +806,7 @@ i40e_status i40e_asq_send_command(struct i40e_hw *hw,
 
 	/* if ready, copy the desc back to temp */
 	if (i40e_asq_done(hw)) {
-		memcpy(desc, desc_on_ring, sizeof(struct i40e_aq_desc));
+		*desc = *desc_on_ring;
 		if (buff != NULL)
 			memcpy(buff, dma_buff->va, buff_size);
 		retval = le16_to_cpu(desc->retval);
diff --git a/drivers/net/ethernet/intel/i40e/i40e_common.c b/drivers/net/ethernet/intel/i40e/i40e_common.c
index c21df7b..1e4ea13 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_common.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_common.c
@@ -507,7 +507,7 @@ i40e_status i40e_aq_get_link_info(struct i40e_hw *hw,
 
 	/* save link status information */
 	if (link)
-		memcpy(link, hw_link_info, sizeof(struct i40e_link_status));
+		*link = *hw_link_info;
 
 	/* flag cleared so helper functions don't call AQ again */
 	hw->phy.get_link_info = false;
-- 
1.8.3.1

^ permalink raw reply related

* [net 5/6] i40e: better return values
From: Jeff Kirsher @ 2013-09-24  9:45 UTC (permalink / raw)
  To: davem; +Cc: Jesse Brandeburg, netdev, gospo, sassmann, Joe Perches,
	Jeff Kirsher
In-Reply-To: <1380015910-25927-1-git-send-email-jeffrey.t.kirsher@intel.com>

From: Jesse Brandeburg <jesse.brandeburg@intel.com>

As mentioned by Joe Perches, clean up return values in some functions
making sure to have consistent return types, not mixing types.

A couple of Joe's comments suggested returning void, but since
the functions in question are ndo defined, the return values are fixed.
So make a comment in the header that notes this is a function called by
net_device_ops.

Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
CC: Joe Perches <joe@perches.com>
Tested-by: Kavindya Deegala <kavindya.s.deegala@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 37 ++++++++++++++++-------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 60c7152..4cbedbd 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -1787,6 +1787,8 @@ int i40e_vsi_add_vlan(struct i40e_vsi *vsi, s16 vid)
  * i40e_vsi_kill_vlan - Remove vsi membership for given vlan
  * @vsi: the vsi being configured
  * @vid: vlan id to be removed (0 = untagged only , -1 = any)
+ *
+ * Return: 0 on success or negative otherwise
  **/
 int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
 {
@@ -1860,37 +1862,39 @@ int i40e_vsi_kill_vlan(struct i40e_vsi *vsi, s16 vid)
  * i40e_vlan_rx_add_vid - Add a vlan id filter to HW offload
  * @netdev: network interface to be adjusted
  * @vid: vlan id to be added
+ *
+ * net_device_ops implementation for adding vlan ids
  **/
 static int i40e_vlan_rx_add_vid(struct net_device *netdev,
 				__always_unused __be16 proto, u16 vid)
 {
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
-	int ret;
+	int ret = 0;
 
 	if (vid > 4095)
-		return 0;
+		return -EINVAL;
+
+	netdev_info(netdev, "adding %pM vid=%d\n", netdev->dev_addr, vid);
 
-	netdev_info(vsi->netdev, "adding %pM vid=%d\n",
-		    netdev->dev_addr, vid);
 	/* If the network stack called us with vid = 0, we should
 	 * indicate to i40e_vsi_add_vlan() that we want to receive
 	 * any traffic (i.e. with any vlan tag, or untagged)
 	 */
 	ret = i40e_vsi_add_vlan(vsi, vid ? vid : I40E_VLAN_ANY);
 
-	if (!ret) {
-		if (vid < VLAN_N_VID)
-			set_bit(vid, vsi->active_vlans);
-	}
+	if (!ret && (vid < VLAN_N_VID))
+		set_bit(vid, vsi->active_vlans);
 
-	return 0;
+	return ret;
 }
 
 /**
  * i40e_vlan_rx_kill_vid - Remove a vlan id filter from HW offload
  * @netdev: network interface to be adjusted
  * @vid: vlan id to be removed
+ *
+ * net_device_ops implementation for adding vlan ids
  **/
 static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
 				 __always_unused __be16 proto, u16 vid)
@@ -1898,15 +1902,16 @@ static int i40e_vlan_rx_kill_vid(struct net_device *netdev,
 	struct i40e_netdev_priv *np = netdev_priv(netdev);
 	struct i40e_vsi *vsi = np->vsi;
 
-	netdev_info(vsi->netdev, "removing %pM vid=%d\n",
-		    netdev->dev_addr, vid);
+	netdev_info(netdev, "removing %pM vid=%d\n", netdev->dev_addr, vid);
+
 	/* return code is ignored as there is nothing a user
 	 * can do about failure to remove and a log message was
-	 * already printed from another function
+	 * already printed from the other function
 	 */
 	i40e_vsi_kill_vlan(vsi, vid);
 
 	clear_bit(vid, vsi->active_vlans);
+
 	return 0;
 }
 
@@ -3324,7 +3329,8 @@ static void i40e_pf_unquiesce_all_vsi(struct i40e_pf *pf)
  **/
 static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
 {
-	int num_tc = 0, i;
+	u8 num_tc = 0;
+	int i;
 
 	/* Scan the ETS Config Priority Table to find
 	 * traffic class enabled for a given priority
@@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
 	/* Traffic class index starts from zero so
 	 * increment to return the actual count
 	 */
-	num_tc++;
-
-	return num_tc;
+	return num_tc++;
 }
 
 /**
@@ -3491,6 +3495,7 @@ static int i40e_vsi_get_bw_info(struct i40e_vsi *vsi)
 		/* 3 bits out of 4 for each TC */
 		vsi->bw_ets_max_quanta[i] = (u8)((tc_bw_max >> (i*4)) & 0x7);
 	}
+
 	return 0;
 }
 
-- 
1.8.3.1

^ permalink raw reply related

* Re: linux-next: manual merge of the ipsec-next tree with the net-next tree
From: Steffen Klassert @ 2013-09-24 10:25 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: linux-next, linux-kernel, Fan Du, Joe Perches, David Miller,
	netdev
In-Reply-To: <20130924121629.f818475ddf308b0494cfce2a@canb.auug.org.au>

Hi Stephen.

On Tue, Sep 24, 2013 at 12:16:29PM +1000, Stephen Rothwell wrote:
> Hi Steffen,
> 
> Today's linux-next merge of the ipsec-next tree got a conflict in
> include/net/xfrm.h between commit d511337a1eda ("xfrm.h: Remove extern
> from function prototypes") from the net-next tree and commit aba826958830
> ("{ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode
> callback") from the ipsec-next tree.
> 

Thanks for the information, I'll do a rebase of the ipsec-next
tree tomorrow.

^ permalink raw reply

* Re: [PATCH 12/19] wireless: Change variable type to bool
From: Kalle Valo @ 2013-09-24 10:54 UTC (permalink / raw)
  To: Peter Senna Tschudin
  Cc: Larry.Finger, chaoming_li, linville, linux-wireless, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <1379802471-30252-12-git-send-email-peter.senna@gmail.com>

Peter Senna Tschudin <peter.senna@gmail.com> writes:

> The variable continual is only assigned the values true and false.
> Change its type to bool.
>
> The simplified semantic patch that find this problem is as
> follows (http://coccinelle.lip6.fr/):
>
> @exists@
> type T;
> identifier b;
> @@
> - T
> + bool
>   b = ...;
>   ... when any
>   b = \(true\|false\)
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> ---
>  drivers/net/wireless/rtlwifi/efuse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Please prefix the patch title with "rtlwifi:". We use "wireless:" for
changes to net/wireless.

-- 
Kalle Valo

^ permalink raw reply

* Re: [net 5/6] i40e: better return values
From: Joe Perches @ 2013-09-24 11:34 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Jesse Brandeburg, netdev, gospo, sassmann
In-Reply-To: <1380015910-25927-6-git-send-email-jeffrey.t.kirsher@intel.com>

On Tue, 2013-09-24 at 02:45 -0700, Jeff Kirsher wrote:

> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
[]
> @@ -3339,9 +3345,7 @@ static u8 i40e_dcb_get_num_tc(struct i40e_dcbx_config *dcbcfg)
>  	/* Traffic class index starts from zero so
>  	 * increment to return the actual count
>  	 */
> -	num_tc++;
> -
> -	return num_tc;
> +	return num_tc++;

Ick.  post_increment problem.

	return ++num_tc;

There's nothing wrong with the original code
unless this is a bugfix which should be documented
better than "better return values".

^ permalink raw reply

* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-24 11:45 UTC (permalink / raw)
  To: vyasevic
  Cc: Toshiaki Makita, David Miller, netdev, Fernando Luis Vazquez Cao,
	Patrick McHardy
In-Reply-To: <524052FC.5010301@redhat.com>

On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich wrote:
> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> > On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich wrote:
> >> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>> On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >>>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >>>> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>>>
> >>>>> There seem to be some undesirable behaviors related with PVID.
> >>>>> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >>>>> to any frame regardless of whether we set it or not.
> >>>>> 2. FDB entries learned via frames applied PVID are registered with
> >>>>> VID 0 rather than VID value of PVID.
> >>>>> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >>>>> This leads interoperational problems such as sending frames with VID
> >>>>> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >>>>> 0 as they belong to VLAN 0, which is expected to be handled as they have
> >>>>> no VID according to IEEE 802.1Q.
> >>>>>
> >>>>> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >>>>> is fixed, because we cannot activate PVID due to it.
> >>>>
> >>>> Please work out the issues in patch #2 with Vlad and resubmit this
> >>>> series.
> >>>>
> >>>> Thank you.
> >>>
> >>> I'm hovering between whether we should fix the issue by changing vlan 0
> >>> interface behavior in 8021q module or enabling a bridge port to sending
> >>> priority-tagged frames, or another better way.
> >>>
> >>> If you could comment it, I'd appreciate it :)
> >>>
> >>>
> >>> BTW, I think what is discussed in patch #2 is another problem about
> >>> handling priority-tags, and it exists without this patch set applied.
> >>> It looks like that we should prepare another patch set than this to fix
> >>> that problem.
> >>>
> >>> Should I include patches that fix the priority-tags problem in this
> >>> patch set and resubmit them all together?
> >>>
> >>
> >> I am thinking that we might need to do it in bridge and it looks like
> >> the simplest way to do it is to have default priority regeneration table
> >> (table 6-5 from 802.1Q doc).
> >>
> >> That way I think we would conform to the spec.
> >>
> >> -vlad
> >
> > Unfortunately I don't think the default priority regeneration table
> > resolves the problem because IEEE 802.1Q says that a VLAN-aware bridge
> > can transmit untagged or VLAN-tagged frames only (the end of section 7.5
> > and 8.1.7).
> >
> > No mechanism to send priority-tagged frames is found as far as I can see
> > the standard. I think the regenerated priority is used for outgoing PCP
> > field only if egress policy is not untagged (i.e. transmitting as
> > VLAN-tagged), and unused if untagged (Section 6.9.2 3rd/4th Paragraph).
> >
> > If we want to transmit priority-tagged frames from a bridge port, I
> > think we need to implement a new (optional) feature that is above the
> > standard, as I stated previously.
> >
> > How do you feel about adding a per-port policy that enables a bridge to
> > send priority-tagged frames instead of untagged frames when egress
> > policy for the port is untagged?
> > With this change, we can transmit frames for a given vlan as either all
> > untagged, all priority-tagged or all VLAN-tagged.
> 
> That would work.  What I am thinking is that we do it by special casing
> the vid 0 egress policy specification.  Let it be untagged by default 
> and if it is tagged, then we preserve the priority field and forward
> it on.
> 
> This keeps the API stable and doesn't require user/admin from knowing 
> exactly what happens.  Default operation conforms to the spec and allows
> simple change to make it backward-compatible.
> 
> What do you think.  I've done a simple prototype of this an it seems to 
> work with the VMs I am testing with.

Are you saying that
- by default, set the 0th bit of untagged_bitmap; and
- if we unset the 0th bit and set the "vid"th bit, we transmit frames
classified as belonging to VLAN "vid" as priority-tagged?

If so, though it's attractive to keep current API, I'm worried about if
it could be a bit confusing and not intuitive for kernel/iproute2
developers that VID 0 has a special meaning only in the egress policy.
Wouldn't it be better to adding a new member to struct net_port_vlans
instead of using VID 0 of untagged_bitmap?

Or are you saying that we use a new flag in struct net_port_vlans but
use the BRIDGE_VLAN_INFO_UNTAGGED bit with VID 0 in netlink to set the
flag?

Even in that case, I'm afraid that it might be confusing for developers
for the same reason. We are going to prohibit to specify VID with 0 (and
4095) in adding/deleting a FDB entry or a vlan filtering entry, but it
would allow us to use VID 0 only when a vlan filtering entry is
configured.
I am thinking a new nlattr is a straightforward approach to configure
it.

Thanks,

Toshiaki Makita

> 
> -vlad
> 
> >
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Toshiaki Makita
> >>>
> >>>>
> >>>> --
> >>>> 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
> >>>
> >>>
> >>>
> >
> >
> > --
> > 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 net-next] xfrm: Simplify SA looking up when using wildcard source address
From: Steffen Klassert @ 2013-09-24 11:45 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1379927917-17365-1-git-send-email-fan.du@windriver.com>

On Mon, Sep 23, 2013 at 05:18:37PM +0800, Fan Du wrote:
> I'm not quite sure I get this "wildcard source address" right,
> IMHO if a host needs to protect every traffic for a given remote host,
> then the source address is wildcard address, i.e. all ZEROs.
> (Please correct me if I'm bloodly wrong。。。)

The above does not belong to a commit message, really.
If you are not sure and you want comments on your patch,
mark your patch as RFC. You should be sure that your patch
is correct when you submit, at least in the moment you
send it. I know that this can change a second after,
but in that moment you should be sure.

> 
> Here is the argument if above statement stands true:
> __xfrm4/6_state_addr_check is a four steps check, all we need to do
> is checking whether the destination address match. Passing saddr from
> flow is worst option, as the checking needs to reach the fourth step.
> 
> So, simply this process by only checking destination address only when
> using wildcard source address for looking up SAs.
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---

If you have further comments on your patch that should not be
included in the commit message, you can add them here.

>  include/net/xfrm.h    |   31 +++++++++++++++++++++++++++++++
>  net/xfrm/xfrm_state.c |    2 +-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index e253bf0..fdb9343 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1282,6 +1282,37 @@ xfrm_state_addr_check(const struct xfrm_state *x,
>  }
>  
>  static __inline__ int
> +__xfrm4_state_daddr_check(const struct xfrm_state *x,
> +                                const xfrm_address_t *daddr)
> +{
> +        return ((daddr->a4 == x->id.daddr.a4) ? 1 : 0);
> +}
> +
> +static __inline__ int
> +__xfrm6_state_daddr_check(const struct xfrm_state *x,
> +                         const xfrm_address_t *daddr)
> +{
> +        if (ipv6_addr_equal((struct in6_addr *)daddr, (struct in6_addr *)&x->id.daddr))
> +                return 1;
> +        else 
> +                return 0;
> +}
> +
> +static __inline__ int
> +xfrm_state_daddr_check(const struct xfrm_state *x,
> +                      const xfrm_address_t *daddr,
> +                      unsigned short family)
> +{
> +        switch (family) {
> +        case AF_INET:
> +                return __xfrm4_state_daddr_check(x, daddr);
> +        case AF_INET6:
> +                return __xfrm6_state_daddr_check(x, daddr);
> +        }    
> +        return 0;
> +}

You used whitespaces where you should use tabs in the whole patch.
Please do the formating right to avoid cleanup patches.

^ 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