Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net] ipv6: add link-local, sit and loopback address with INFINITY_LIFE_TIME
From: Damien Wyart @ 2014-01-08 16:12 UTC (permalink / raw)
  To: François-Xavier Le Bail, netdev, davem, yasushi.asano
In-Reply-To: <20140108144322.GJ9007@order.stressinduktion.org>

* Hannes Frederic Sowa <hannes@stressinduktion.org> [2014-01-08 15:43]:
> In the past the IFA_PERMANENT flag indicated, that the valid and preferred
> lifetime where ignored. Since change fad8da3e085ddf ("ipv6 addrconf: fix
> preferred lifetime state-changing behavior while valid_lft is infinity")
> we honour at least the preferred lifetime on those addresses. As such
> the valid lifetime gets recalculated and updated to 0.

> If loopback address is added manually this problem does not occur.
> Also if NetworkManager manages IPv6, those addresses will get added via
> inet6_rtm_newaddr and thus will have a correct lifetime, too.

I confirm this patch (on top of 3.13-rc7+) fixes the problem on my
system, eth0 has a LLA adress again after boot and ::1 is back on the
loopback interface.

Thanks for the quick fix!

-- 
Damien Wyart

^ permalink raw reply

* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Or Gerlitz @ 2014-01-08 16:11 UTC (permalink / raw)
  To: Tom Herbert, Or Gerlitz
  Cc: Jerry Chu, Eric Dumazet, Herbert Xu, Linux Netdev List,
	David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CA+mtBx-HRi4ttijScF8DKtdooeOyjzPXwq7T69vNaPSodZh1dQ@mail.gmail.com>

On 08/01/2014 01:04, Tom Herbert wrote:
> On Tue, Jan 7, 2014 at 12:21 PM, Or Gerlitz<or.gerlitz@gmail.com>  wrote:
>> >On Tue, Jan 7, 2014 at 8:44 PM, Tom Herbert<therbert@google.com>  wrote:
>>> >>Or, thanks for posting the patches!
>>> >>
>>> >>We should also support the case where direct encapsulation is being
>>> >>done, that is there is no encapsulation header after UDP and the
>>> >>protocol of the encapsulated packet is inferred by the port number
>>> >>(e.g. GRE/UDP, TCP/UDP, SCTP/UDP, etc.). This is probably an
>>> >>additional field in net_offload struct for next protocol, a little
>>> >>more API, and pretty trivial handlers in UDP code.
>> >
>> >The way I have set that follows your guideline under which the
>> >encapsulating method is derived from the udp destination port in the
>> >sense that the encapsulating protocol can do what they want in the
>> >gro_receive/complete handlers entry they plant per that udp port,
>> >isn't that generic enough?
> Direct encapsulation of different protocols could be done using the
> same callback functions. Somehow, we just need to pass the protocol
> number (or more generally pass private data to the callbacks). In lieu
> adding this to net_offload, we could just make the net_offload an
> argument to the callbacks and use container_of to access a private structure.
>

Tom,

OK, so I understand that you want the infrastructure to allow for direct 
udp encapsulation in the sense that there is no specific encapsulation 
header following the udp header.

Take for example encapsulating TCP over UDP port X, the layer/driver 
that wants to enable gro for such traffic will register a udp gro 
handler for port X and in their gro receive/complete
callbacks would just assume that following the UDP header there's TCP 
header and they will then issue a lookup in the inet gro handlers array 
to get the TCP gro handler, problem solved, agree?

Or.

^ permalink raw reply

* [PATCH net-next] packet: improve socket create/bind latency in some cases
From: Daniel Borkmann @ 2014-01-08 16:12 UTC (permalink / raw)
  To: davem; +Cc: netdev

Most people acquire PF_PACKET sockets with a protocol argument in
the socket call, e.g. libpcap does so with htons(ETH_P_ALL) for
all its sockets. Most likely, at some point in time a subsequent
bind() call will follow, e.g. in libpcap with ...

  memset(&sll, 0, sizeof(sll));
  sll.sll_family          = AF_PACKET;
  sll.sll_ifindex         = ifindex;
  sll.sll_protocol        = htons(ETH_P_ALL);

... as arguments. What happens in the kernel is that already
in socket() syscall, we install a proto hook via register_prot_hook()
if our protocol argument is != 0. Yet, in bind() we're almost
doing the same work by doing a unregister_prot_hook() with an
expensive synchronize_net() call in case during socket() the proto
was != 0, plus follow-up register_prot_hook() with a bound device
to it this time, in order to limit traffic we get.

In the case when the protocol and user supplied device index (== 0)
does not change from socket() to bind(), we can spare us doing
the same work twice. Similarly for re-binding to the same device
and protocol. For these scenarios, we can decrease create/bind
latency from ~7447us (sock-bind-2 case) to ~89us (sock-bind-1 case)
with this patch.

Alternatively, for the first case, if people care, they should
simply create their sockets with proto == 0 argument and define
the protocol during bind() as this saves a call to synchronize_net()
as well (sock-bind-3 case).

In all other cases, we're tied to user space behaviour we must not
change, also since a bind() is not strictly required. Thus, we need
the synchronize_net() to make sure no asynchronous packet processing
paths still refer to the previous elements of po->prot_hook.

In case of mmap()ed sockets, the workflow that includes bind() is
socket() -> setsockopt(<ring>) -> bind(). In that case, a pair of
{__unregister, register}_prot_hook is being called from setsockopt()
in order to install the new protocol receive handler. Thus, when
we call bind and can skip a re-hook, we have already previously
installed the new handler. For fanout, this is handled different
entirely, so we should be good.

Timings on an i7-3520M machine:

  * sock-bind-1:   89 us
  * sock-bind-2: 7447 us
  * sock-bind-3:   75 us

sock-bind-1:
  socket(PF_PACKET, SOCK_RAW, htons(ETH_P_IP)) = 3
  bind(3, {sa_family=AF_PACKET, proto=htons(ETH_P_IP), if=all(0),
           pkttype=PACKET_HOST, addr(0)={0, }, 20) = 0

sock-bind-2:
  socket(PF_PACKET, SOCK_RAW, htons(ETH_P_IP)) = 3
  bind(3, {sa_family=AF_PACKET, proto=htons(ETH_P_IP), if=lo(1),
           pkttype=PACKET_HOST, addr(0)={0, }, 20) = 0

sock-bind-3:
  socket(PF_PACKET, SOCK_RAW, 0) = 3
  bind(3, {sa_family=AF_PACKET, proto=htons(ETH_P_IP), if=lo(1),
           pkttype=PACKET_HOST, addr(0)={0, }, 20) = 0

While at it, also make it a bit more readable.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/packet/af_packet.c | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 279467b..a65f5b0 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2567,9 +2567,12 @@ static int packet_release(struct socket *sock)
  *	Attach a packet hook.
  */
 
-static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 protocol)
+static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 proto)
 {
 	struct packet_sock *po = pkt_sk(sk);
+	__be16 proto_curr = po->prot_hook.type;
+	const struct net_device *dev_curr = po->prot_hook.dev;
+	bool need_rehook = proto_curr != proto || dev_curr != dev;
 
 	if (po->fanout) {
 		if (dev)
@@ -2579,21 +2582,24 @@ static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 protoc
 	}
 
 	lock_sock(sk);
-
 	spin_lock(&po->bind_lock);
-	unregister_prot_hook(sk, true);
 
-	po->num = protocol;
-	po->prot_hook.type = protocol;
-	if (po->prot_hook.dev)
-		dev_put(po->prot_hook.dev);
+	if (need_rehook) {
+		unregister_prot_hook(sk, true);
+
+		po->num = proto;
+		po->prot_hook.type = proto;
+
+		if (po->prot_hook.dev)
+			dev_put(po->prot_hook.dev);
 
-	po->prot_hook.dev = dev;
-	po->ifindex = dev ? dev->ifindex : 0;
+		po->prot_hook.dev = dev;
 
-	packet_cached_dev_assign(po, dev);
+		po->ifindex = dev ? dev->ifindex : 0;
+		packet_cached_dev_assign(po, dev);
+	}
 
-	if (protocol == 0)
+	if (proto == 0 || !need_rehook)
 		goto out_unlock;
 
 	if (!dev || (dev->flags & IFF_UP)) {
@@ -2693,22 +2699,22 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
 
 	err = -ENOBUFS;
 	sk = sk_alloc(net, PF_PACKET, GFP_KERNEL, &packet_proto);
-	if (sk == NULL)
+	if (unlikely(sk == NULL))
 		goto out;
 
 	sock->ops = &packet_ops;
-	if (sock->type == SOCK_PACKET)
+	if (unlikely(sock->type == SOCK_PACKET))
 		sock->ops = &packet_ops_spkt;
 
 	sock_init_data(sock, sk);
 
 	po = pkt_sk(sk);
-	sk->sk_family = PF_PACKET;
 	po->num = proto;
 	po->xmit = dev_queue_xmit;
 
 	packet_cached_dev_reset(po);
 
+	sk->sk_family = PF_PACKET;
 	sk->sk_destruct = packet_sock_destruct;
 	sk_refcnt_debug_inc(sk);
 
@@ -2718,9 +2724,9 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
 
 	spin_lock_init(&po->bind_lock);
 	mutex_init(&po->pg_vec_lock);
-	po->prot_hook.func = packet_rcv;
 
-	if (sock->type == SOCK_PACKET)
+	po->prot_hook.func = packet_rcv;
+	if (unlikely(sock->type == SOCK_PACKET))
 		po->prot_hook.func = packet_rcv_spkt;
 
 	po->prot_hook.af_packet_priv = sk;
-- 
1.8.3.1

^ permalink raw reply related

* Re: [PATCH v3.6 08/19] ozwpan: slight optimization of addr compare
From: Rupesh Gujare @ 2014-01-08 16:04 UTC (permalink / raw)
  To: Ding Tianhong, Greg Kroah-Hartman, devel, Netdev,
	linux-kernel@vger.kernel.org
In-Reply-To: <52CCBDB0.9060709@huawei.com>

On 08/01/14 02:53, Ding Tianhong wrote:
> Use possibly more efficient ether_addr_equal
> instead of memcmp.
>
> Cc: Rupesh Gujare <rupesh.gujare@atmel.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: devel@driverdev.osuosl.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Tan Xiaojun <tanxiaojun@huawei.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
>   drivers/staging/ozwpan/ozcdev.c  | 2 +-
>   drivers/staging/ozwpan/ozproto.c | 5 +++--
>   2 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
> index 6ce0af9..c363f66 100644
> --- a/drivers/staging/ozwpan/ozcdev.c
> +++ b/drivers/staging/ozwpan/ozcdev.c
> @@ -448,7 +448,7 @@ int oz_cdev_start(struct oz_pd *pd, int resume)
>   	}
>   	spin_lock(&g_cdev.lock);
>   	if ((g_cdev.active_pd == NULL) &&
> -		(memcmp(pd->mac_addr, g_cdev.active_addr, ETH_ALEN) == 0)) {
> +		ether_addr_equal(pd->mac_addr, g_cdev.active_addr)) {
>   		oz_pd_get(pd);
>   		g_cdev.active_pd = pd;
>   		oz_dbg(ON, "Active PD arrived\n");
> diff --git a/drivers/staging/ozwpan/ozproto.c b/drivers/staging/ozwpan/ozproto.c
> index 88714ec..19a2521 100644
> --- a/drivers/staging/ozwpan/ozproto.c
> +++ b/drivers/staging/ozwpan/ozproto.c
> @@ -9,6 +9,7 @@
>   #include <linux/timer.h>
>   #include <linux/sched.h>
>   #include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
>   #include <linux/errno.h>
>   #include <linux/ieee80211.h>
>   #include "ozdbg.h"
> @@ -180,7 +181,7 @@ static struct oz_pd *oz_connect_req(struct oz_pd *cur_pd, struct oz_elt *elt,
>   		spin_lock_bh(&g_polling_lock);
>   		list_for_each(e, &g_pd_list) {
>   			pd2 = container_of(e, struct oz_pd, link);
> -			if (memcmp(pd2->mac_addr, pd_addr, ETH_ALEN) == 0) {
> +			if (ether_addr_equal(pd2->mac_addr, pd_addr)) {
>   				free_pd = pd;
>   				pd = pd2;
>   				break;
> @@ -597,7 +598,7 @@ struct oz_pd *oz_pd_find(const u8 *mac_addr)
>   	spin_lock_bh(&g_polling_lock);
>   	list_for_each(e, &g_pd_list) {
>   		pd = container_of(e, struct oz_pd, link);
> -		if (memcmp(pd->mac_addr, mac_addr, ETH_ALEN) == 0) {
> +		if (ether_addr_equal(pd->mac_addr, mac_addr)) {
>   			atomic_inc(&pd->ref_count);
>   			spin_unlock_bh(&g_polling_lock);
>   			return pd;

Acked-by: Rupesh Gujare <rupesh.gujare@atmel.com>

Thanks,

-- 
Regards,
Rupesh Gujare

^ permalink raw reply

* Re: Use of 'SIOCDEVPRIVATE' in ethernet drivers.
From: Giri Reddy @ 2014-01-08 16:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, netdev
In-Reply-To: <20140107154456.7e5af377@nehalam.linuxnetplumber.net>


Thanks for the input. I will explore the 'ethtool' option. We will have
multiple control frames exchanged between user space and kernel for
every flash update operation, most of this will be proprietary stuff
that will get exchanged - I will explore how to provide a generic
interface for that.

Giri

> Maybe a debugfs or sysfs filesystem would work better than adding ioctl';s

IOCTL option is off the table based on feedback from DaveM. I'm
considering 'sysfs' as an option along with 'ethtool', making this
generic without too much proprietary taint will be the problem that
needs to be solved.

Giri

>


________________________________

This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message.

^ permalink raw reply

* [PATCH net-next 3/3] bonding: remove dead code from 3ad
From: Veaceslav Falico @ 2014-01-08 15:46 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389196008-28578-1-git-send-email-vfalico@redhat.com>

That code has been around for ages without being used.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_3ad.c | 78 ------------------------------------------
 1 file changed, 78 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 7118eef..29db1ca 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -594,40 +594,6 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 }
 
 /**
- * __attach_bond_to_agg
- * @port: the port we're looking at
- *
- * Handle the attaching of the port's control parser/multiplexer and the
- * aggregator. This function does nothing since the parser/multiplexer of the
- * receive and the parser/multiplexer of the aggregator are already combined.
- */
-static void __attach_bond_to_agg(struct port *port)
-{
-	port = NULL; /* just to satisfy the compiler */
-	/* This function does nothing since the parser/multiplexer of the
-	 * receive and the parser/multiplexer of the aggregator are already
-	 * combined.
-	 */
-}
-
-/**
- * __detach_bond_from_agg
- * @port: the port we're looking at
- *
- * Handle the detaching of the port's control parser/multiplexer from the
- * aggregator. This function does nothing since the parser/multiplexer of the
- * receive and the parser/multiplexer of the aggregator are already combined.
- */
-static void __detach_bond_from_agg(struct port *port)
-{
-	port = NULL; /* just to satisfy the compiler */
-	/* This function does nothing since the parser/multiplexer of the
-	 * receive and the parser/multiplexer of the aggregator are already
-	 * combined
-	 */
-}
-
-/**
  * __agg_ports_are_ready - check if all ports in an aggregator are ready
  * @aggregator: the aggregator we're looking at
  *
@@ -964,7 +930,6 @@ static void ad_mux_machine(struct port *port)
 			 port->sm_mux_state);
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
-			__detach_bond_from_agg(port);
 			port->actor_oper_port_state &= ~AD_STATE_SYNCHRONIZATION;
 			ad_disable_collecting_distributing(port);
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
@@ -975,7 +940,6 @@ static void ad_mux_machine(struct port *port)
 			port->sm_mux_timer_counter = __ad_timer_to_ticks(AD_WAIT_WHILE_TIMER, 0);
 			break;
 		case AD_MUX_ATTACHED:
-			__attach_bond_to_agg(port);
 			port->actor_oper_port_state |= AD_STATE_SYNCHRONIZATION;
 			port->actor_oper_port_state &= ~AD_STATE_COLLECTING;
 			port->actor_oper_port_state &= ~AD_STATE_DISTRIBUTING;
@@ -1774,48 +1738,6 @@ static void ad_disable_collecting_distributing(struct port *port)
 	}
 }
 
-#if 0
-/**
- * ad_marker_info_send - send a marker information frame
- * @port: the port we're looking at
- *
- * This function does nothing since we decided not to implement send and handle
- * response for marker PDU's, in this stage, but only to respond to marker
- * information.
- */
-static void ad_marker_info_send(struct port *port)
-{
-	struct bond_marker marker;
-	u16 index;
-
-	// fill the marker PDU with the appropriate values
-	marker.subtype = 0x02;
-	marker.version_number = 0x01;
-	marker.tlv_type = AD_MARKER_INFORMATION_SUBTYPE;
-	marker.marker_length = 0x16;
-	// convert requester_port to Big Endian
-	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |((u16)(port->actor_port_number & 0xFF00) >> 8));
-	marker.requester_system = port->actor_system;
-	// convert requester_port(u32) to Big Endian
-	marker.requester_transaction_id =
-		(((++port->transaction_id & 0xFF) << 24)
-		 | ((port->transaction_id & 0xFF00) << 8)
-		 | ((port->transaction_id & 0xFF0000) >> 8)
-		 | ((port->transaction_id & 0xFF000000) >> 24));
-	marker.pad = 0;
-	marker.tlv_type_terminator = 0x00;
-	marker.terminator_length = 0x00;
-	for (index = 0; index < 90; index++)
-		marker.reserved_90[index] = 0;
-
-	// send the marker information
-	if (ad_marker_send(port, &marker) >= 0) {
-		pr_debug("Sent Marker Information on port %d\n",
-			 port->actor_port_number);
-	}
-}
-#endif
-
 /**
  * ad_marker_info_received - handle receive of a Marker information frame
  * @marker_info: Marker info received
-- 
1.8.4

^ permalink raw reply related

* [PATCH net-next 2/3] bonding: convert 3ad to use pr_warn instead of pr_warning
From: Veaceslav Falico @ 2014-01-08 15:46 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389196008-28578-1-git-send-email-vfalico@redhat.com>

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_3ad.c | 50 +++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 9828209..7118eef 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -1318,11 +1318,11 @@ static void ad_port_selection_logic(struct port *port)
 			/* meaning: the port was related to an aggregator
 			 * but was not on the aggregator port list
 			 */
-			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
-				   port->slave->bond->dev->name,
-				   port->actor_port_number,
-				   port->slave->dev->name,
-				   port->aggregator->aggregator_identifier);
+			pr_warn("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
+				port->slave->bond->dev->name,
+				port->actor_port_number,
+				port->slave->dev->name,
+				port->aggregator->aggregator_identifier);
 		}
 	}
 	/* search on all aggregators for a suitable aggregator for this port */
@@ -1479,9 +1479,9 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 		break;
 
 	default:
-		pr_warning("%s: Impossible agg select mode %d\n",
-			   curr->slave->bond->dev->name,
-			   __get_agg_selection_mode(curr->lag_ports));
+		pr_warn("%s: Impossible agg select mode %d\n",
+			curr->slave->bond->dev->name,
+			__get_agg_selection_mode(curr->lag_ports));
 		break;
 	}
 
@@ -1594,7 +1594,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 
 		/* check if any partner replys */
 		if (best->is_individual) {
-			pr_warning("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
+			pr_warn("%s: Warning: No 802.3ad response from the link partner for any adapters in the bond\n",
 				best->slave ?
 				best->slave->bond->dev->name : "NULL");
 		}
@@ -1986,8 +1986,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
-			   slave->bond->dev->name, slave->dev->name);
+		pr_warn("Warning: %s: Trying to unbind an uninitialized port on %s\n",
+			slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2064,8 +2064,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				if (select_new_active_agg)
 					ad_agg_selection_logic(__get_first_agg(port));
 			} else {
-				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
-					   slave->bond->dev->name);
+				pr_warn("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
+					slave->bond->dev->name);
 			}
 		} else {
 			/* in case that the only port related to this
@@ -2158,8 +2158,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		/* select the active aggregator for the bond */
 		if (port) {
 			if (!port->slave) {
-				pr_warning("%s: Warning: bond's first port is uninitialized\n",
-					   bond->dev->name);
+				pr_warn("%s: Warning: bond's first port is uninitialized\n",
+					bond->dev->name);
 				goto re_arm;
 			}
 
@@ -2173,8 +2173,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	bond_for_each_slave_rcu(bond, slave, iter) {
 		port = &(SLAVE_AD_INFO(slave).port);
 		if (!port->slave) {
-			pr_warning("%s: Warning: Found an uninitialized port\n",
-				   bond->dev->name);
+			pr_warn("%s: Warning: Found an uninitialized port\n",
+				bond->dev->name);
 			goto re_arm;
 		}
 
@@ -2224,8 +2224,8 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave,
 		port = &(SLAVE_AD_INFO(slave).port);
 
 		if (!port->slave) {
-			pr_warning("%s: Warning: port of slave %s is uninitialized\n",
-				   slave->dev->name, slave->bond->dev->name);
+			pr_warn("%s: Warning: port of slave %s is uninitialized\n",
+				slave->dev->name, slave->bond->dev->name);
 			return ret;
 		}
 
@@ -2282,8 +2282,8 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: speed changed for uninitialized port on %s\n",
-			   slave->bond->dev->name, slave->dev->name);
+		pr_warn("Warning: %s: speed changed for uninitialized port on %s\n",
+			slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2315,8 +2315,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("%s: Warning: duplex changed for uninitialized port on %s\n",
-			   slave->bond->dev->name, slave->dev->name);
+		pr_warn("%s: Warning: duplex changed for uninitialized port on %s\n",
+			slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
@@ -2349,8 +2349,8 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 
 	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
-		pr_warning("Warning: %s: link status changed for uninitialized port on %s\n",
-			   slave->bond->dev->name, slave->dev->name);
+		pr_warn("Warning: %s: link status changed for uninitialized port on %s\n",
+			slave->bond->dev->name, slave->dev->name);
 		return;
 	}
 
-- 
1.8.4

^ permalink raw reply related

* [PATCH net-next 0/3] bonding: cleanup bond_3ad.c
From: Veaceslav Falico @ 2014-01-08 15:46 UTC (permalink / raw)
  To: netdev; +Cc: Jay Vosburgh, Andy Gospodarek, Veaceslav Falico

It's a huge mess there currently - and, thus, really hard to read and
debug.

This is the first series, and doesn't change the logic at all, only makes
it a bit more readable.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>

---
 drivers/net/bonding/bond_3ad.c | 731 +++++++++++++++++++++--------------------
 1 file changed, 366 insertions(+), 365 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/3] bonding: clean up style for bond_3ad.c
From: Veaceslav Falico @ 2014-01-08 15:46 UTC (permalink / raw)
  To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389196008-28578-1-git-send-email-vfalico@redhat.com>

It's a huge mess currently, that is really hard to read. This cleanup
doesn't touch the logic at all, it only breaks easy-to-fix long lines and
updates comment styles.

CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
 drivers/net/bonding/bond_3ad.c | 619 +++++++++++++++++++++++------------------
 1 file changed, 349 insertions(+), 270 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 539e24a..9828209 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -34,14 +34,14 @@
 #include "bonding.h"
 #include "bond_3ad.h"
 
-// General definitions
+/* General definitions */
 #define AD_SHORT_TIMEOUT           1
 #define AD_LONG_TIMEOUT            0
 #define AD_STANDBY                 0x2
 #define AD_MAX_TX_IN_SECOND        3
 #define AD_COLLECTOR_MAX_DELAY     0
 
-// Timer definitions(43.4.4 in the 802.3ad standard)
+/* Timer definitions (43.4.4 in the 802.3ad standard) */
 #define AD_FAST_PERIODIC_TIME      1
 #define AD_SLOW_PERIODIC_TIME      30
 #define AD_SHORT_TIMEOUT_TIME      (3*AD_FAST_PERIODIC_TIME)
@@ -49,7 +49,7 @@
 #define AD_CHURN_DETECTION_TIME    60
 #define AD_AGGREGATE_WAIT_TIME     2
 
-// Port state definitions(43.4.2.2 in the 802.3ad standard)
+/* Port state definitions (43.4.2.2 in the 802.3ad standard) */
 #define AD_STATE_LACP_ACTIVITY   0x1
 #define AD_STATE_LACP_TIMEOUT    0x2
 #define AD_STATE_AGGREGATION     0x4
@@ -59,7 +59,9 @@
 #define AD_STATE_DEFAULTED       0x40
 #define AD_STATE_EXPIRED         0x80
 
-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
+/* Port Variables definitions used by the State Machines (43.4.7 in the
+ * 802.3ad standard)
+ */
 #define AD_PORT_BEGIN           0x1
 #define AD_PORT_LACP_ENABLED    0x2
 #define AD_PORT_ACTOR_CHURN     0x4
@@ -71,24 +73,23 @@
 #define AD_PORT_SELECTED        0x100
 #define AD_PORT_MOVED           0x200
 
-// Port Key definitions
-// key is determined according to the link speed, duplex and
-// user key(which is yet not supported)
-//              ------------------------------------------------------------
-// Port key :   | User key                       |      Speed       |Duplex|
-//              ------------------------------------------------------------
-//              16                               6               1 0
+/* Port Key definitions
+ * key is determined according to the link speed, duplex and
+ * user key (which is yet not supported)
+ * --------------------------------------------------------------
+ * Port key :	| User key	| Speed		| Duplex	|
+ * --------------------------------------------------------------
+ * 16		  6		  1		  0
+ */
 #define  AD_DUPLEX_KEY_BITS    0x1
 #define  AD_SPEED_KEY_BITS     0x3E
 #define  AD_USER_KEY_BITS      0xFFC0
 
-//dalloun
 #define     AD_LINK_SPEED_BITMASK_1MBPS       0x1
 #define     AD_LINK_SPEED_BITMASK_10MBPS      0x2
 #define     AD_LINK_SPEED_BITMASK_100MBPS     0x4
 #define     AD_LINK_SPEED_BITMASK_1000MBPS    0x8
 #define     AD_LINK_SPEED_BITMASK_10000MBPS   0x10
-//endalloun
 
 /* compare MAC addresses */
 #define MAC_ADDRESS_EQUAL(A, B)	\
@@ -100,7 +101,7 @@ static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
 static const u8 lacpdu_mcast_addr[ETH_ALEN] = MULTICAST_LACPDU_ADDR;
 
-// ================= main 802.3ad protocol functions ==================
+/* ================= main 802.3ad protocol functions ================== */
 static int ad_lacpdu_send(struct port *port);
 static int ad_marker_send(struct port *port, struct bond_marker *marker);
 static void ad_mux_machine(struct port *port);
@@ -114,13 +115,13 @@ static void ad_initialize_agg(struct aggregator *aggregator);
 static void ad_initialize_port(struct port *port, int lacp_fast);
 static void ad_enable_collecting_distributing(struct port *port);
 static void ad_disable_collecting_distributing(struct port *port);
-static void ad_marker_info_received(struct bond_marker *marker_info, struct port *port);
-static void ad_marker_response_received(struct bond_marker *marker, struct port *port);
+static void ad_marker_info_received(struct bond_marker *marker_info,
+				    struct port *port);
+static void ad_marker_response_received(struct bond_marker *marker,
+					struct port *port);
 
 
-/////////////////////////////////////////////////////////////////////////////////
-// ================= api to bonding and kernel code ==================
-/////////////////////////////////////////////////////////////////////////////////
+/* ================= api to bonding and kernel code ================== */
 
 /**
  * __get_bond_by_port - get the port's bonding struct
@@ -151,17 +152,20 @@ static inline struct aggregator *__get_first_agg(struct port *port)
 	/* If there's no bond for this port, or bond has no slaves */
 	if (bond == NULL)
 		return NULL;
+
 	rcu_read_lock();
 	first_slave = bond_first_slave_rcu(bond);
 	rcu_read_unlock();
+
 	return first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL;
 }
 
-/*
- * __agg_has_partner
+/**
+ * __agg_has_partner - see if we have a partner
+ * @agg: the agregator we're looking at
  *
  * Return nonzero if aggregator has a partner (denoted by a non-zero ether
- * address for the partner).  Return 0 if not.
+ * address for the partner). Return 0 if not.
  */
 static inline int __agg_has_partner(struct aggregator *agg)
 {
@@ -171,7 +175,6 @@ static inline int __agg_has_partner(struct aggregator *agg)
 /**
  * __disable_port - disable the port's slave
  * @port: the port we're looking at
- *
  */
 static inline void __disable_port(struct port *port)
 {
@@ -181,7 +184,6 @@ static inline void __disable_port(struct port *port)
 /**
  * __enable_port - enable the port's slave, if it's up
  * @port: the port we're looking at
- *
  */
 static inline void __enable_port(struct port *port)
 {
@@ -194,7 +196,6 @@ static inline void __enable_port(struct port *port)
 /**
  * __port_is_enabled - check if the port's slave is in active state
  * @port: the port we're looking at
- *
  */
 static inline int __port_is_enabled(struct port *port)
 {
@@ -220,7 +221,6 @@ static inline u32 __get_agg_selection_mode(struct port *port)
 /**
  * __check_agg_selection_timer - check if the selection timer has expired
  * @port: the port we're looking at
- *
  */
 static inline int __check_agg_selection_timer(struct port *port)
 {
@@ -235,7 +235,6 @@ static inline int __check_agg_selection_timer(struct port *port)
 /**
  * __get_state_machine_lock - lock the port's state machines
  * @port: the port we're looking at
- *
  */
 static inline void __get_state_machine_lock(struct port *port)
 {
@@ -245,7 +244,6 @@ static inline void __get_state_machine_lock(struct port *port)
 /**
  * __release_state_machine_lock - unlock the port's state machines
  * @port: the port we're looking at
- *
  */
 static inline void __release_state_machine_lock(struct port *port)
 {
@@ -268,10 +266,11 @@ static u16 __get_link_speed(struct port *port)
 	struct slave *slave = port->slave;
 	u16 speed;
 
-	/* this if covers only a special case: when the configuration starts with
-	 * link down, it sets the speed to 0.
-	 * This is done in spite of the fact that the e100 driver reports 0 to be
-	 * compatible with MVT in the future.*/
+	/* this if covers only a special case: when the configuration starts
+	 * with link down, it sets the speed to 0.
+	 * This is done in spite of the fact that the e100 driver reports 0
+	 * to be compatible with MVT in the future.
+	 */
 	if (slave->link != BOND_LINK_UP)
 		speed = 0;
 	else {
@@ -293,7 +292,8 @@ static u16 __get_link_speed(struct port *port)
 			break;
 
 		default:
-			speed = 0; // unknown speed value from ethtool. shouldn't happen
+			/* unknown speed value from ethtool. shouldn't happen */
+			speed = 0;
 			break;
 		}
 	}
@@ -317,8 +317,9 @@ static u8 __get_duplex(struct port *port)
 
 	u8 retval;
 
-	//  handling a special case: when the configuration starts with
-	// link down, it sets the duplex to 0.
+	/* handling a special case: when the configuration starts with
+	 * link down, it sets the duplex to 0.
+	 */
 	if (slave->link != BOND_LINK_UP)
 		retval = 0x0;
 	else {
@@ -342,15 +343,14 @@ static u8 __get_duplex(struct port *port)
 /**
  * __initialize_port_locks - initialize a port's STATE machine spinlock
  * @port: the slave of the port we're looking at
- *
  */
 static inline void __initialize_port_locks(struct slave *slave)
 {
-	// make sure it isn't called twice
+	/* make sure it isn't called twice */
 	spin_lock_init(&(SLAVE_AD_INFO(slave).state_machine_lock));
 }
 
-//conversions
+/* Conversions */
 
 /**
  * __ad_timer_to_ticks - convert a given timer type to AD module ticks
@@ -359,39 +359,38 @@ static inline void __initialize_port_locks(struct slave *slave)
  *
  * If @timer_type is %current_while_timer, @par indicates long/short timer.
  * If @timer_type is %periodic_timer, @par is one of %FAST_PERIODIC_TIME,
- *						    %SLOW_PERIODIC_TIME.
+ *						     %SLOW_PERIODIC_TIME.
  */
 static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 {
 	u16 retval = 0; /* to silence the compiler */
 
 	switch (timer_type) {
-	case AD_CURRENT_WHILE_TIMER:   // for rx machine usage
+	case AD_CURRENT_WHILE_TIMER:	/* for rx machine usage */
 		if (par)
-			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec); // short timeout
+			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec);
 		else
-			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec); // long timeout
+			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec);
 		break;
-	case AD_ACTOR_CHURN_TIMER:	    // for local churn machine
+	case AD_ACTOR_CHURN_TIMER:	/* for local churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_PERIODIC_TIMER:	    // for periodic machine
-		retval = (par*ad_ticks_per_sec); // long timeout
+	case AD_PERIODIC_TIMER:		/* for periodic machine */
+		retval = (par*ad_ticks_per_sec); /* long timeout */
 		break;
-	case AD_PARTNER_CHURN_TIMER:   // for remote churn machine
+	case AD_PARTNER_CHURN_TIMER:	/* for remote churn machine */
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
 		break;
-	case AD_WAIT_WHILE_TIMER:	    // for selection machine
+	case AD_WAIT_WHILE_TIMER:	/* for selection machine */
 		retval = (AD_AGGREGATE_WAIT_TIME*ad_ticks_per_sec);
 		break;
 	}
+
 	return retval;
 }
 
 
-/////////////////////////////////////////////////////////////////////////////////
-// ================= ad_rx_machine helper functions ==================
-/////////////////////////////////////////////////////////////////////////////////
+/* ================= ad_rx_machine helper functions ================== */
 
 /**
  * __choose_matched - update a port's matched variable from a received lacpdu
@@ -451,7 +450,9 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		struct port_params *partner = &port->partner_oper;
 
 		__choose_matched(lacpdu, port);
-		// record the new parameter values for the partner operational
+		/* record the new parameter values for the partner
+		 * operational
+		 */
 		partner->port_number = ntohs(lacpdu->actor_port);
 		partner->port_priority = ntohs(lacpdu->actor_port_priority);
 		partner->system = lacpdu->actor_system;
@@ -459,10 +460,12 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		partner->key = ntohs(lacpdu->actor_key);
 		partner->port_state = lacpdu->actor_state;
 
-		// set actor_oper_port_state.defaulted to FALSE
+		/* set actor_oper_port_state.defaulted to FALSE */
 		port->actor_oper_port_state &= ~AD_STATE_DEFAULTED;
 
-		// set the partner sync. to on if the partner is sync. and the port is matched
+		/* set the partner sync. to on if the partner is sync,
+		 * and the port is matched
+		 */
 		if ((port->sm_vars & AD_PORT_MATCHED)
 		    && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
@@ -482,11 +485,11 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 static void __record_default(struct port *port)
 {
 	if (port) {
-		// record the partner admin parameters
+		/* record the partner admin parameters */
 		memcpy(&port->partner_oper, &port->partner_admin,
 		       sizeof(struct port_params));
 
-		// set actor_oper_port_state.defaulted to true
+		/* set actor_oper_port_state.defaulted to true */
 		port->actor_oper_port_state |= AD_STATE_DEFAULTED;
 	}
 }
@@ -601,8 +604,10 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 static void __attach_bond_to_agg(struct port *port)
 {
 	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing since the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	/* This function does nothing since the parser/multiplexer of the
+	 * receive and the parser/multiplexer of the aggregator are already
+	 * combined.
+	 */
 }
 
 /**
@@ -616,8 +621,10 @@ static void __attach_bond_to_agg(struct port *port)
 static void __detach_bond_from_agg(struct port *port)
 {
 	port = NULL; /* just to satisfy the compiler */
-	// This function does nothing since the parser/multiplexer of the receive
-	// and the parser/multiplexer of the aggregator are already combined
+	/* This function does nothing since the parser/multiplexer of the
+	 * receive and the parser/multiplexer of the aggregator are already
+	 * combined
+	 */
 }
 
 /**
@@ -631,7 +638,9 @@ static int __agg_ports_are_ready(struct aggregator *aggregator)
 	int retval = 1;
 
 	if (aggregator) {
-		// scan all ports in this aggregator to verfy if they are all ready
+		/* scan all ports in this aggregator to verfy if they are
+		 * all ready.
+		 */
 		for (port = aggregator->lag_ports;
 		     port;
 		     port = port->next_port_in_aggregator) {
@@ -691,7 +700,7 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 			bandwidth = aggregator->num_of_ports * 10000;
 			break;
 		default:
-			bandwidth = 0; /*to silence the compiler ....*/
+			bandwidth = 0; /* to silence the compiler */
 		}
 	}
 	return bandwidth;
@@ -700,7 +709,6 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 /**
  * __get_active_agg - get the current active aggregator
  * @aggregator: the aggregator we're looking at
- *
  */
 static struct aggregator *__get_active_agg(struct aggregator *aggregator)
 {
@@ -722,15 +730,14 @@ static struct aggregator *__get_active_agg(struct aggregator *aggregator)
 /**
  * __update_lacpdu_from_port - update a port's lacpdu fields
  * @port: the port we're looking at
- *
  */
 static inline void __update_lacpdu_from_port(struct port *port)
 {
 	struct lacpdu *lacpdu = &port->lacpdu;
 	const struct port_params *partner = &port->partner_oper;
 
-	/* update current actual Actor parameters */
-	/* lacpdu->subtype                   initialized
+	/* update current actual Actor parameters
+	 * lacpdu->subtype                   initialized
 	 * lacpdu->version_number            initialized
 	 * lacpdu->tlv_type_actor_info       initialized
 	 * lacpdu->actor_information_length  initialized
@@ -766,9 +773,7 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	 */
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= main 802.3ad protocol code ======================================
-//////////////////////////////////////////////////////////////////////////////////////
+/* ================= main 802.3ad protocol code ========================= */
 
 /**
  * ad_lacpdu_send - send out a lacpdu packet on a given port
@@ -798,11 +803,12 @@ static int ad_lacpdu_send(struct port *port)
 
 	memcpy(lacpdu_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
 	/* Note: source address is set to be the member's PERMANENT address,
-	   because we use it to identify loopback lacpdus in receive. */
+	 * because we use it to identify loopback lacpdus in receive.
+	 */
 	memcpy(lacpdu_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	lacpdu_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	lacpdu_header->lacpdu = port->lacpdu; // struct copy
+	lacpdu_header->lacpdu = port->lacpdu;
 
 	dev_queue_xmit(skb);
 
@@ -839,11 +845,12 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 
 	memcpy(marker_header->hdr.h_dest, lacpdu_mcast_addr, ETH_ALEN);
 	/* Note: source address is set to be the member's PERMANENT address,
-	   because we use it to identify loopback MARKERs in receive. */
+	 * because we use it to identify loopback MARKERs in receive.
+	 */
 	memcpy(marker_header->hdr.h_source, slave->perm_hwaddr, ETH_ALEN);
 	marker_header->hdr.h_proto = PKT_TYPE_LACPDU;
 
-	marker_header->marker = *marker; // struct copy
+	marker_header->marker = *marker;
 
 	dev_queue_xmit(skb);
 
@@ -853,72 +860,90 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 /**
  * ad_mux_machine - handle a port's mux state machine
  * @port: the port we're looking at
- *
  */
 static void ad_mux_machine(struct port *port)
 {
 	mux_states_t last_state;
 
-	// keep current State Machine state to compare later if it was changed
+	/* keep current State Machine state to compare later if it was
+	 * changed
+	 */
 	last_state = port->sm_mux_state;
 
 	if (port->sm_vars & AD_PORT_BEGIN) {
-		port->sm_mux_state = AD_MUX_DETACHED;		 // next state
+		port->sm_mux_state = AD_MUX_DETACHED;
 	} else {
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
 			if ((port->sm_vars & AD_PORT_SELECTED)
 			    || (port->sm_vars & AD_PORT_STANDBY))
 				/* if SELECTED or STANDBY */
-				port->sm_mux_state = AD_MUX_WAITING; // next state
+				port->sm_mux_state = AD_MUX_WAITING;
 			break;
 		case AD_MUX_WAITING:
-			// if SELECTED == FALSE return to DETACH state
-			if (!(port->sm_vars & AD_PORT_SELECTED)) { // if UNSELECTED
+			/* if SELECTED == FALSE return to DETACH state */
+			if (!(port->sm_vars & AD_PORT_SELECTED)) {
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the Selection Logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
+				/* in order to withhold the Selection Logic to
+				 * check all ports READY_N value every callback
+				 * cycle to update ready variable, we check
+				 * READY_N and update READY here
+				 */
 				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;	 // next state
+				port->sm_mux_state = AD_MUX_DETACHED;
 				break;
 			}
 
-			// check if the wait_while_timer expired
+			/* check if the wait_while_timer expired */
 			if (port->sm_mux_timer_counter
 			    && !(--port->sm_mux_timer_counter))
 				port->sm_vars |= AD_PORT_READY_N;
 
-			// in order to withhold the selection logic to check all ports READY_N value
-			// every callback cycle to update ready variable, we check READY_N and update READY here
+			/* in order to withhold the selection logic to check
+			 * all ports READY_N value every callback cycle to
+			 * update ready variable, we check READY_N and update
+			 * READY here
+			 */
 			__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
 
-			// if the wait_while_timer expired, and the port is in READY state, move to ATTACHED state
+			/* if the wait_while_timer expired, and the port is
+			 * in READY state, move to ATTACHED state
+			 */
 			if ((port->sm_vars & AD_PORT_READY)
 			    && !port->sm_mux_timer_counter)
-				port->sm_mux_state = AD_MUX_ATTACHED;	 // next state
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			break;
 		case AD_MUX_ATTACHED:
-			// check also if agg_select_timer expired(so the edable port will take place only after this timer)
-			if ((port->sm_vars & AD_PORT_SELECTED) && (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) && !__check_agg_selection_timer(port)) {
-				port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;// next state
-			} else if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY)) {	  // if UNSELECTED or STANDBY
+			/* check also if agg_select_timer expired (so the
+			 * edable port will take place only after this timer)
+			 */
+			if ((port->sm_vars & AD_PORT_SELECTED) &&
+			    (port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION) &&
+			    !__check_agg_selection_timer(port)) {
+				port->sm_mux_state = AD_MUX_COLLECTING_DISTRIBUTING;
+			} else if (!(port->sm_vars & AD_PORT_SELECTED) ||
+				   (port->sm_vars & AD_PORT_STANDBY)) {
+				/* if UNSELECTED or STANDBY */
 				port->sm_vars &= ~AD_PORT_READY_N;
-				// in order to withhold the selection logic to check all ports READY_N value
-				// every callback cycle to update ready variable, we check READY_N and update READY here
+				/* in order to withhold the selection logic to
+				 * check all ports READY_N value every callback
+				 * cycle to update ready variable, we check
+				 * READY_N and update READY here
+				 */
 				__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
-				port->sm_mux_state = AD_MUX_DETACHED;// next state
+				port->sm_mux_state = AD_MUX_DETACHED;
 			}
 			break;
 		case AD_MUX_COLLECTING_DISTRIBUTING:
-			if (!(port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY) ||
-			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)
-			   ) {
-				port->sm_mux_state = AD_MUX_ATTACHED;// next state
-
+			if (!(port->sm_vars & AD_PORT_SELECTED) ||
+			    (port->sm_vars & AD_PORT_STANDBY) ||
+			    !(port->partner_oper.port_state & AD_STATE_SYNCHRONIZATION)) {
+				port->sm_mux_state = AD_MUX_ATTACHED;
 			} else {
-				// if port state hasn't changed make
-				// sure that a collecting distributing
-				// port in an active aggregator is enabled
+				/* if port state hasn't changed make
+				 * sure that a collecting distributing
+				 * port in an active aggregator is enabled
+				 */
 				if (port->aggregator &&
 				    port->aggregator->is_active &&
 				    !__port_is_enabled(port)) {
@@ -927,12 +952,12 @@ static void ad_mux_machine(struct port *port)
 				}
 			}
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_mux_state != last_state) {
 		pr_debug("Mux Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -963,7 +988,7 @@ static void ad_mux_machine(struct port *port)
 			ad_enable_collecting_distributing(port);
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
@@ -982,59 +1007,63 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 {
 	rx_states_t last_state;
 
-	// keep current State Machine state to compare later if it was changed
+	/* keep current State Machine state to compare later if it was
+	 * changed
+	 */
 	last_state = port->sm_rx_state;
 
-	// check if state machine should change state
-	// first, check if port was reinitialized
+	/* check if state machine should change state */
+
+	/* first, check if port was reinitialized */
 	if (port->sm_vars & AD_PORT_BEGIN)
-		/* next state */
 		port->sm_rx_state = AD_RX_INITIALIZE;
-	// check if port is not enabled
+	/* check if port is not enabled */
 	else if (!(port->sm_vars & AD_PORT_BEGIN)
 		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
-		/* next state */
 		port->sm_rx_state = AD_RX_PORT_DISABLED;
-	// check if new lacpdu arrived
-	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) || (port->sm_rx_state == AD_RX_DEFAULTED) || (port->sm_rx_state == AD_RX_CURRENT))) {
-		port->sm_rx_timer_counter = 0; // zero timer
+	/* check if new lacpdu arrived */
+	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) ||
+		 (port->sm_rx_state == AD_RX_DEFAULTED) ||
+		 (port->sm_rx_state == AD_RX_CURRENT))) {
+		port->sm_rx_timer_counter = 0;
 		port->sm_rx_state = AD_RX_CURRENT;
 	} else {
-		// if timer is on, and if it is expired
-		if (port->sm_rx_timer_counter && !(--port->sm_rx_timer_counter)) {
+		/* if timer is on, and if it is expired */
+		if (port->sm_rx_timer_counter &&
+		    !(--port->sm_rx_timer_counter)) {
 			switch (port->sm_rx_state) {
 			case AD_RX_EXPIRED:
-				port->sm_rx_state = AD_RX_DEFAULTED;		// next state
+				port->sm_rx_state = AD_RX_DEFAULTED;
 				break;
 			case AD_RX_CURRENT:
-				port->sm_rx_state = AD_RX_EXPIRED;	    // next state
+				port->sm_rx_state = AD_RX_EXPIRED;
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 			}
 		} else {
-			// if no lacpdu arrived and no timer is on
+			/* if no lacpdu arrived and no timer is on */
 			switch (port->sm_rx_state) {
 			case AD_RX_PORT_DISABLED:
 				if (port->sm_vars & AD_PORT_MOVED)
-					port->sm_rx_state = AD_RX_INITIALIZE;	    // next state
+					port->sm_rx_state = AD_RX_INITIALIZE;
 				else if (port->is_enabled
 					 && (port->sm_vars
 					     & AD_PORT_LACP_ENABLED))
-					port->sm_rx_state = AD_RX_EXPIRED;	// next state
+					port->sm_rx_state = AD_RX_EXPIRED;
 				else if (port->is_enabled
 					 && ((port->sm_vars
 					      & AD_PORT_LACP_ENABLED) == 0))
-					port->sm_rx_state = AD_RX_LACP_DISABLED;    // next state
+					port->sm_rx_state = AD_RX_LACP_DISABLED;
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 
 			}
 		}
 	}
 
-	// check if the State machine was changed or new lacpdu arrived
+	/* check if the State machine was changed or new lacpdu arrived */
 	if ((port->sm_rx_state != last_state) || (lacpdu)) {
 		pr_debug("Rx Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
@@ -1049,10 +1078,9 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			__record_default(port);
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			port->sm_vars &= ~AD_PORT_MOVED;
-			port->sm_rx_state = AD_RX_PORT_DISABLED;	// next state
-
-			/*- Fall Through -*/
+			port->sm_rx_state = AD_RX_PORT_DISABLED;
 
+			/* Fall Through */
 		case AD_RX_PORT_DISABLED:
 			port->sm_vars &= ~AD_PORT_MATCHED;
 			break;
@@ -1064,13 +1092,15 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
 		case AD_RX_EXPIRED:
-			//Reset of the Synchronization flag. (Standard 43.4.12)
-			//This reset cause to disable this port in the COLLECTING_DISTRIBUTING state of the
-			//mux machine in case of EXPIRED even if LINK_DOWN didn't arrive for the port.
+			/* Reset of the Synchronization flag (Standard 43.4.12)
+			 * This reset cause to disable this port in the
+			 * COLLECTING_DISTRIBUTING state of the mux machine in
+			 * case of EXPIRED even if LINK_DOWN didn't arrive for
+			 * the port.
+			 */
 			port->partner_oper.port_state &= ~AD_STATE_SYNCHRONIZATION;
 			port->sm_vars &= ~AD_PORT_MATCHED;
-			port->partner_oper.port_state |=
-				AD_STATE_LACP_ACTIVITY;
+			port->partner_oper.port_state |= AD_STATE_LACP_ACTIVITY;
 			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(AD_SHORT_TIMEOUT));
 			port->actor_oper_port_state |= AD_STATE_EXPIRED;
 			break;
@@ -1082,10 +1112,11 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			break;
 		case AD_RX_CURRENT:
 			/* detect loopback situation */
-			if (MAC_ADDRESS_EQUAL(&(lacpdu->actor_system), &(port->actor_system))) {
-				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
-				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
-				       port->slave->bond->dev->name, port->slave->dev->name);
+			if (MAC_ADDRESS_EQUAL(&(lacpdu->actor_system),
+					      &(port->actor_system))) {
+				pr_err("%s: An illegal loopback occurred on adapter (%s).\nCheck the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
+				       port->slave->bond->dev->name,
+				       port->slave->dev->name);
 				return;
 			}
 			__update_selected(lacpdu, port);
@@ -1094,7 +1125,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			port->sm_rx_timer_counter = __ad_timer_to_ticks(AD_CURRENT_WHILE_TIMER, (u16)(port->actor_oper_port_state & AD_STATE_LACP_TIMEOUT));
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
 			break;
-		default:    /* to silence the compiler */
+		default:
 			break;
 		}
 	}
@@ -1103,13 +1134,14 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 /**
  * ad_tx_machine - handle a port's tx state machine
  * @port: the port we're looking at
- *
  */
 static void ad_tx_machine(struct port *port)
 {
-	// check if tx timer expired, to verify that we do not send more than 3 packets per second
+	/* check if tx timer expired, to verify that we do not send more than
+	 * 3 packets per second
+	 */
 	if (port->sm_tx_timer_counter && !(--port->sm_tx_timer_counter)) {
-		// check if there is something to send
+		/* check if there is something to send */
 		if (port->ntt && (port->sm_vars & AD_PORT_LACP_ENABLED)) {
 			__update_lacpdu_from_port(port);
 
@@ -1117,14 +1149,16 @@ static void ad_tx_machine(struct port *port)
 				pr_debug("Sent LACPDU on port %d\n",
 					 port->actor_port_number);
 
-				/* mark ntt as false, so it will not be sent again until
-				   demanded */
+				/* mark ntt as false, so it will not be sent
+				 * again until demanded
+				 */
 				port->ntt = false;
 			}
 		}
-		// restart tx timer(to verify that we will not exceed AD_MAX_TX_IN_SECOND
-		port->sm_tx_timer_counter =
-			ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
+		/* restart tx timer(to verify that we will not exceed
+		 * AD_MAX_TX_IN_SECOND
+		 */
+		port->sm_tx_timer_counter = ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 	}
 }
 
@@ -1138,76 +1172,79 @@ static void ad_periodic_machine(struct port *port)
 {
 	periodic_states_t last_state;
 
-	// keep current state machine state to compare later if it was changed
+	/* keep current state machine state to compare later if it was changed */
 	last_state = port->sm_periodic_state;
 
-	// check if port was reinitialized
+	/* check if port was reinitialized */
 	if (((port->sm_vars & AD_PORT_BEGIN) || !(port->sm_vars & AD_PORT_LACP_ENABLED) || !port->is_enabled) ||
 	    (!(port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY) && !(port->partner_oper.port_state & AD_STATE_LACP_ACTIVITY))
 	   ) {
-		port->sm_periodic_state = AD_NO_PERIODIC;	     // next state
+		port->sm_periodic_state = AD_NO_PERIODIC;
 	}
-	// check if state machine should change state
+	/* check if state machine should change state */
 	else if (port->sm_periodic_timer_counter) {
-		// check if periodic state machine expired
+		/* check if periodic state machine expired */
 		if (!(--port->sm_periodic_timer_counter)) {
-			// if expired then do tx
-			port->sm_periodic_state = AD_PERIODIC_TX;    // next state
+			/* if expired then do tx */
+			port->sm_periodic_state = AD_PERIODIC_TX;
 		} else {
-			// If not expired, check if there is some new timeout parameter from the partner state
+			/* If not expired, check if there is some new timeout
+			 * parameter from the partner state
+			 */
 			switch (port->sm_periodic_state) {
 			case AD_FAST_PERIODIC:
 				if (!(port->partner_oper.port_state
 				      & AD_STATE_LACP_TIMEOUT))
-					port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+					port->sm_periodic_state = AD_SLOW_PERIODIC;
 				break;
 			case AD_SLOW_PERIODIC:
 				if ((port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
-					// stop current timer
 					port->sm_periodic_timer_counter = 0;
-					port->sm_periodic_state = AD_PERIODIC_TX;	 // next state
+					port->sm_periodic_state = AD_PERIODIC_TX;
 				}
 				break;
-			default:    //to silence the compiler
+			default:
 				break;
 			}
 		}
 	} else {
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_state = AD_FAST_PERIODIC;	 // next state
+			port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
 		case AD_PERIODIC_TX:
-			if (!(port->partner_oper.port_state
-			      & AD_STATE_LACP_TIMEOUT))
-				port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
+			if (!(port->partner_oper.port_state &
+			    AD_STATE_LACP_TIMEOUT))
+				port->sm_periodic_state = AD_SLOW_PERIODIC;
 			else
-				port->sm_periodic_state = AD_FAST_PERIODIC;  // next state
+				port->sm_periodic_state = AD_FAST_PERIODIC;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
 
-	// check if the state machine was changed
+	/* check if the state machine was changed */
 	if (port->sm_periodic_state != last_state) {
 		pr_debug("Periodic Machine: Port=%d, Last State=%d, Curr State=%d\n",
 			 port->actor_port_number, last_state,
 			 port->sm_periodic_state);
 		switch (port->sm_periodic_state) {
 		case AD_NO_PERIODIC:
-			port->sm_periodic_timer_counter = 0;	   // zero timer
+			port->sm_periodic_timer_counter = 0;
 			break;
 		case AD_FAST_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_FAST_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in the PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_FAST_PERIODIC_TIME))-1;
 			break;
 		case AD_SLOW_PERIODIC:
-			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1; // decrement 1 tick we lost in the PERIODIC_TX cycle
+			/* decrement 1 tick we lost in the PERIODIC_TX cycle */
+			port->sm_periodic_timer_counter = __ad_timer_to_ticks(AD_PERIODIC_TIMER, (u16)(AD_SLOW_PERIODIC_TIME))-1;
 			break;
 		case AD_PERIODIC_TX:
 			port->ntt = true;
 			break;
-		default:    //to silence the compiler
+		default:
 			break;
 		}
 	}
@@ -1230,30 +1267,38 @@ static void ad_port_selection_logic(struct port *port)
 	struct slave *slave;
 	int found = 0;
 
-	// if the port is already Selected, do nothing
+	/* if the port is already Selected, do nothing */
 	if (port->sm_vars & AD_PORT_SELECTED)
 		return;
 
 	bond = __get_bond_by_port(port);
 
-	// if the port is connected to other aggregator, detach it
+	/* if the port is connected to other aggregator, detach it */
 	if (port->aggregator) {
-		// detach the port from its former aggregator
+		/* detach the port from its former aggregator */
 		temp_aggregator = port->aggregator;
 		for (curr_port = temp_aggregator->lag_ports; curr_port;
 		     last_port = curr_port,
-			     curr_port = curr_port->next_port_in_aggregator) {
+		     curr_port = curr_port->next_port_in_aggregator) {
 			if (curr_port == port) {
 				temp_aggregator->num_of_ports--;
-				if (!last_port) {// if it is the first port attached to the aggregator
+				/* if it is the first port attached to the
+				 * aggregator
+				 */
+				if (!last_port) {
 					temp_aggregator->lag_ports =
 						port->next_port_in_aggregator;
-				} else {// not the first port attached to the aggregator
+				} else {
+					/* not the first port attached to the
+					 * aggregator
+					 */
 					last_port->next_port_in_aggregator =
 						port->next_port_in_aggregator;
 				}
 
-				// clear the port's relations to this aggregator
+				/* clear the port's relations to this
+				 * aggregator
+				 */
 				port->aggregator = NULL;
 				port->next_port_in_aggregator = NULL;
 				port->actor_port_aggregator_identifier = 0;
@@ -1261,13 +1306,18 @@ static void ad_port_selection_logic(struct port *port)
 				pr_debug("Port %d left LAG %d\n",
 					 port->actor_port_number,
 					 temp_aggregator->aggregator_identifier);
-				// if the aggregator is empty, clear its parameters, and set it ready to be attached
+				/* if the aggregator is empty, clear its
+				 * parameters, and set it ready to be attached
+				 */
 				if (!temp_aggregator->lag_ports)
 					ad_clear_agg(temp_aggregator);
 				break;
 			}
 		}
-		if (!curr_port) { // meaning: the port was related to an aggregator but was not on the aggregator port list
+		if (!curr_port) {
+			/* meaning: the port was related to an aggregator
+			 * but was not on the aggregator port list
+			 */
 			pr_warning("%s: Warning: Port %d (on %s) was related to aggregator %d but was not on its port list\n",
 				   port->slave->bond->dev->name,
 				   port->actor_port_number,
@@ -1275,11 +1325,11 @@ static void ad_port_selection_logic(struct port *port)
 				   port->aggregator->aggregator_identifier);
 		}
 	}
-	// search on all aggregators for a suitable aggregator for this port
+	/* search on all aggregators for a suitable aggregator for this port */
 	bond_for_each_slave(bond, slave, iter) {
 		aggregator = &(SLAVE_AD_INFO(slave).aggregator);
 
-		// keep a free aggregator for later use(if needed)
+		/* keep a free aggregator for later use(if needed) */
 		if (!aggregator->lag_ports) {
 			if (!free_aggregator)
 				free_aggregator = aggregator;
@@ -1306,23 +1356,26 @@ static void ad_port_selection_logic(struct port *port)
 				 port->actor_port_number,
 				 port->aggregator->aggregator_identifier);
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 			found = 1;
 			break;
 		}
 	}
 
-	// the port couldn't find an aggregator - attach it to a new aggregator
+	/* the port couldn't find an aggregator - attach it to a new
+	 * aggregator
+	 */
 	if (!found) {
 		if (free_aggregator) {
-			// assign port a new aggregator
+			/* assign port a new aggregator */
 			port->aggregator = free_aggregator;
 			port->actor_port_aggregator_identifier =
 				port->aggregator->aggregator_identifier;
 
-			// update the new aggregator's parameters
-			// if port was responsed from the end-user
+			/* update the new aggregator's parameters
+			 * if port was responsed from the end-user
+			 */
 			if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)
 				/* if port is full duplex */
 				port->aggregator->is_individual = false;
@@ -1341,7 +1394,7 @@ static void ad_port_selection_logic(struct port *port)
 			port->aggregator->lag_ports = port;
 			port->aggregator->num_of_ports++;
 
-			// mark this port as selected
+			/* mark this port as selected */
 			port->sm_vars |= AD_PORT_SELECTED;
 
 			pr_debug("Port %d joined LAG %d(new LAG)\n",
@@ -1353,23 +1406,24 @@ static void ad_port_selection_logic(struct port *port)
 			       port->actor_port_number, port->slave->dev->name);
 		}
 	}
-	// if all aggregator's ports are READY_N == TRUE, set ready=TRUE in all aggregator's ports
-	// else set ready=FALSE in all aggregator's ports
-	__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
+	/* if all aggregator's ports are READY_N == TRUE, set ready=TRUE
+	 * in all aggregator's ports, else set ready=FALSE in all
+	 * aggregator's ports
+	 */
+	__set_agg_ports_ready(port->aggregator,
+			      __agg_ports_are_ready(port->aggregator));
 
 	aggregator = __get_first_agg(port);
 	ad_agg_selection_logic(aggregator);
 }
 
-/*
- * Decide if "agg" is a better choice for the new active aggregator that
+/* Decide if "agg" is a better choice for the new active aggregator that
  * the current best, according to the ad_select policy.
  */
 static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 						struct aggregator *curr)
 {
-	/*
-	 * 0. If no best, select current.
+	/* 0. If no best, select current.
 	 *
 	 * 1. If the current agg is not individual, and the best is
 	 *    individual, select current.
@@ -1437,10 +1491,12 @@ static struct aggregator *ad_agg_selection_test(struct aggregator *best,
 static int agg_device_up(const struct aggregator *agg)
 {
 	struct port *port = agg->lag_ports;
+
 	if (!port)
 		return 0;
-	return (netif_running(port->slave->dev) &&
-		netif_carrier_ok(port->slave->dev));
+
+	return netif_running(port->slave->dev) &&
+	       netif_carrier_ok(port->slave->dev);
 }
 
 /**
@@ -1492,8 +1548,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 
 	if (best &&
 	    __get_agg_selection_mode(best->lag_ports) == BOND_AD_STABLE) {
-		/*
-		 * For the STABLE policy, don't replace the old active
+		/* For the STABLE policy, don't replace the old active
 		 * aggregator if it's still active (it has an answering
 		 * partner) or if both the best and active don't have an
 		 * answering partner.
@@ -1501,7 +1556,8 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		if (active && active->lag_ports &&
 		    active->lag_ports->is_enabled &&
 		    (__agg_has_partner(active) ||
-		     (!__agg_has_partner(active) && !__agg_has_partner(best)))) {
+		     (!__agg_has_partner(active) &&
+		     !__agg_has_partner(best)))) {
 			if (!(!active->actor_oper_aggregator_key &&
 			      best->actor_oper_aggregator_key)) {
 				best = NULL;
@@ -1552,7 +1608,9 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 			 best->partner_oper_aggregator_key,
 			 best->is_individual, best->is_active);
 
-		/* disable the ports that were related to the former active_aggregator */
+		/* disable the ports that were related to the former
+		 * active_aggregator
+		 */
 		if (active) {
 			for (port = active->lag_ports; port;
 			     port = port->next_port_in_aggregator) {
@@ -1561,8 +1619,7 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 		}
 	}
 
-	/*
-	 * if the selected aggregator is of join individuals
+	/* if the selected aggregator is of join individuals
 	 * (partner_system is NULL), enable their ports
 	 */
 	active = __get_active_agg(origin);
@@ -1584,7 +1641,6 @@ static void ad_agg_selection_logic(struct aggregator *agg)
 /**
  * ad_clear_agg - clear a given aggregator's parameters
  * @aggregator: the aggregator we're looking at
- *
  */
 static void ad_clear_agg(struct aggregator *aggregator)
 {
@@ -1608,7 +1664,6 @@ static void ad_clear_agg(struct aggregator *aggregator)
 /**
  * ad_initialize_agg - initialize a given aggregator's parameters
  * @aggregator: the aggregator we're looking at
- *
  */
 static void ad_initialize_agg(struct aggregator *aggregator)
 {
@@ -1625,7 +1680,6 @@ static void ad_initialize_agg(struct aggregator *aggregator)
  * ad_initialize_port - initialize a given port's parameters
  * @aggregator: the aggregator we're looking at
  * @lacp_fast: boolean. whether fast periodic should be used
- *
  */
 static void ad_initialize_port(struct port *port, int lacp_fast)
 {
@@ -1657,8 +1711,10 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		port->ntt = false;
 		port->actor_admin_port_key = 1;
 		port->actor_oper_port_key  = 1;
-		port->actor_admin_port_state = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
-		port->actor_oper_port_state  = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
+		port->actor_admin_port_state = AD_STATE_AGGREGATION |
+					       AD_STATE_LACP_ACTIVITY;
+		port->actor_oper_port_state  = AD_STATE_AGGREGATION |
+					       AD_STATE_LACP_ACTIVITY;
 
 		if (lacp_fast)
 			port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
@@ -1667,7 +1723,7 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
 
 		port->is_enabled = true;
-		// ****** private parameters ******
+		/* private parameters */
 		port->sm_vars = 0x3;
 		port->sm_rx_state = 0;
 		port->sm_rx_timer_counter = 0;
@@ -1705,11 +1761,12 @@ static void ad_enable_collecting_distributing(struct port *port)
 /**
  * ad_disable_collecting_distributing - disable a port's transmit/receive
  * @port: the port we're looking at
- *
  */
 static void ad_disable_collecting_distributing(struct port *port)
 {
-	if (port->aggregator && !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system), &(null_mac_addr))) {
+	if (port->aggregator &&
+	    !MAC_ADDRESS_EQUAL(&(port->aggregator->partner_system),
+			       &(null_mac_addr))) {
 		pr_debug("Disabling port %d(LAG %d)\n",
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
@@ -1763,20 +1820,18 @@ static void ad_marker_info_send(struct port *port)
  * ad_marker_info_received - handle receive of a Marker information frame
  * @marker_info: Marker info received
  * @port: the port we're looking at
- *
  */
 static void ad_marker_info_received(struct bond_marker *marker_info,
 	struct port *port)
 {
 	struct bond_marker marker;
 
-	// copy the received marker data to the response marker
-	//marker = *marker_info;
+	/* copy the received marker data to the response marker */
 	memcpy(&marker, marker_info, sizeof(struct bond_marker));
-	// change the marker subtype to marker response
+	/* change the marker subtype to marker response */
 	marker.tlv_type = AD_MARKER_RESPONSE_SUBTYPE;
-	// send the marker response
 
+	/* send the marker response */
 	if (ad_marker_send(port, &marker) >= 0) {
 		pr_debug("Sent Marker Response on port %d\n",
 			 port->actor_port_number);
@@ -1793,22 +1848,21 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
  * information.
  */
 static void ad_marker_response_received(struct bond_marker *marker,
-	struct port *port)
+					struct port *port)
 {
-	marker = NULL; /* just to satisfy the compiler */
-	port = NULL;  /* just to satisfy the compiler */
-	// DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW
+	marker = NULL;
+	port = NULL;
+	/* DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW */
 }
 
-//////////////////////////////////////////////////////////////////////////////////////
-// ================= AD exported functions to the main bonding code ==================
-//////////////////////////////////////////////////////////////////////////////////////
+/* ========= AD exported functions to the main bonding code ========= */
 
-// Check aggregators status in team every T seconds
+/* Check aggregators status in team every T seconds */
 #define AD_AGGREGATOR_SELECTION_TIMER  8
 
-/*
- * bond_3ad_initiate_agg_selection(struct bonding *bond)
+/**
+ * bond_3ad_initiate_agg_selection - initate aggregator selection
+ * @bond: bonding struct
  *
  * Set the aggregation selection timer, to initiate an agg selection in
  * the very near future.  Called during first initialization, and during
@@ -1839,7 +1893,9 @@ void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution)
 		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
 		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
 
-		// initialize how many times this module is called in one second(should be about every 100ms)
+		/* initialize how many times this module is called in one
+		 * second (should be about every 100ms)
+		 */
 		ad_ticks_per_sec = tick_resolution;
 
 		bond_3ad_initiate_agg_selection(bond,
@@ -1874,21 +1930,21 @@ void bond_3ad_bind_slave(struct slave *slave)
 		port->actor_port_number = SLAVE_AD_INFO(slave).id;
 		/* key is determined according to the link speed, duplex and user key(which
 		 * is yet not supported)
-		 *              ------------------------------------------------------------
-		 * Port key :   | User key                       |      Speed       |Duplex|
-		 *              ------------------------------------------------------------
-		 *              16                               6               1 0
 		 */
-		port->actor_admin_port_key = 0;	/* initialize this parameter */
+		port->actor_admin_port_key = 0;
 		port->actor_admin_port_key |= __get_duplex(port);
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
-		/* if the port is not full duplex, then the port should be not lacp Enabled */
+		/* if the port is not full duplex, then the port should be not
+		 * lacp Enabled
+		 */
 		if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS))
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
 		/* actor system is the bond's system */
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
-		/* tx timer(to verify that no more than MAX_TX_IN_SECOND lacpdu's are sent in one second) */
+		/* tx timer(to verify that no more than MAX_TX_IN_SECOND
+		 * lacpdu's are sent in one second)
+		 */
 		port->sm_tx_timer_counter = ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 		port->aggregator = NULL;
 		port->next_port_in_aggregator = NULL;
@@ -1925,13 +1981,10 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	struct slave *slave_iter;
 	struct list_head *iter;
 
-	// find the aggregator related to this slave
 	aggregator = &(SLAVE_AD_INFO(slave).aggregator);
-
-	// find the port related to this slave
 	port = &(SLAVE_AD_INFO(slave).port);
 
-	// if slave is null, the whole port is not initialized
+	/* if slave is null, the whole port is not initialized */
 	if (!port->slave) {
 		pr_warning("Warning: %s: Trying to unbind an uninitialized port on %s\n",
 			   slave->bond->dev->name, slave->dev->name);
@@ -1946,34 +1999,42 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	__update_lacpdu_from_port(port);
 	ad_lacpdu_send(port);
 
-	// check if this aggregator is occupied
+	/* check if this aggregator is occupied */
 	if (aggregator->lag_ports) {
-		// check if there are other ports related to this aggregator except
-		// the port related to this slave(thats ensure us that there is a
-		// reason to search for new aggregator, and that we will find one
-		if ((aggregator->lag_ports != port) || (aggregator->lag_ports->next_port_in_aggregator)) {
-			// find new aggregator for the related port(s)
+		/* check if there are other ports related to this aggregator
+		 * except the port related to this slave(thats ensure us that
+		 * there is a reason to search for new aggregator, and that we
+		 * will find one
+		 */
+		if ((aggregator->lag_ports != port) ||
+		    (aggregator->lag_ports->next_port_in_aggregator)) {
+			/* find new aggregator for the related port(s) */
 			bond_for_each_slave(bond, slave_iter, iter) {
 				new_aggregator = &(SLAVE_AD_INFO(slave_iter).aggregator);
-				// if the new aggregator is empty, or it is connected to our port only
-				if (!new_aggregator->lag_ports
-				    || ((new_aggregator->lag_ports == port)
-					&& !new_aggregator->lag_ports->next_port_in_aggregator))
+				/* if the new aggregator is empty, or it is
+				 * connected to our port only
+				 */
+				if (!new_aggregator->lag_ports ||
+				    ((new_aggregator->lag_ports == port) &&
+				     !new_aggregator->lag_ports->next_port_in_aggregator))
 					break;
 			}
 			if (!slave_iter)
 				new_aggregator = NULL;
-			// if new aggregator found, copy the aggregator's parameters
-			// and connect the related lag_ports to the new aggregator
+
+			/* if new aggregator found, copy the aggregator's
+			 * parameters and connect the related lag_ports to the
+			 * new aggregator
+			 */
 			if ((new_aggregator) && ((!new_aggregator->lag_ports) || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator))) {
 				pr_debug("Some port(s) related to LAG %d - replaceing with LAG %d\n",
 					 aggregator->aggregator_identifier,
 					 new_aggregator->aggregator_identifier);
 
-				if ((new_aggregator->lag_ports == port) && new_aggregator->is_active) {
+				if ((new_aggregator->lag_ports == port) &&
+				    new_aggregator->is_active) {
 					pr_info("%s: Removing an active aggregator\n",
 						aggregator->slave->bond->dev->name);
-					// select new active aggregator
 					 select_new_active_agg = 1;
 				}
 
@@ -1989,14 +2050,15 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				new_aggregator->is_active = aggregator->is_active;
 				new_aggregator->num_of_ports = aggregator->num_of_ports;
 
-				// update the information that is written on the ports about the aggregator
+				/* update the information that is written on
+				 * the ports about the aggregator
+				 */
 				for (temp_port = aggregator->lag_ports; temp_port;
 				     temp_port = temp_port->next_port_in_aggregator) {
 					temp_port->aggregator = new_aggregator;
 					temp_port->actor_port_aggregator_identifier = new_aggregator->aggregator_identifier;
 				}
 
-				// clear the aggregator
 				ad_clear_agg(aggregator);
 
 				if (select_new_active_agg)
@@ -2005,14 +2067,16 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
 					   slave->bond->dev->name);
 			}
-		} else { // in case that the only port related to this aggregator is the one we want to remove
+		} else {
+			/* in case that the only port related to this
+			 * aggregator is the one we want to remove
+			 */
 			select_new_active_agg = aggregator->is_active;
-			// clear the aggregator
 			ad_clear_agg(aggregator);
 			if (select_new_active_agg) {
 				pr_info("%s: Removing an active aggregator\n",
 					slave->bond->dev->name);
-				// select new active aggregator
+				/* select new active aggregator */
 				temp_aggregator = __get_first_agg(port);
 				if (temp_aggregator)
 					ad_agg_selection_logic(temp_aggregator);
@@ -2021,15 +2085,19 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	}
 
 	pr_debug("Unbinding port %d\n", port->actor_port_number);
-	// find the aggregator that this port is connected to
+
+	/* find the aggregator that this port is connected to */
 	bond_for_each_slave(bond, slave_iter, iter) {
 		temp_aggregator = &(SLAVE_AD_INFO(slave_iter).aggregator);
 		prev_port = NULL;
-		// search the port in the aggregator's related ports
+		/* search the port in the aggregator's related ports */
 		for (temp_port = temp_aggregator->lag_ports; temp_port;
 		     prev_port = temp_port,
-			     temp_port = temp_port->next_port_in_aggregator) {
-			if (temp_port == port) { // the aggregator found - detach the port from this aggregator
+		     temp_port = temp_port->next_port_in_aggregator) {
+			if (temp_port == port) {
+				/* the aggregator found - detach the port from
+				 * this aggregator
+				 */
 				if (prev_port)
 					prev_port->next_port_in_aggregator = temp_port->next_port_in_aggregator;
 				else
@@ -2037,12 +2105,11 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				temp_aggregator->num_of_ports--;
 				if (temp_aggregator->num_of_ports == 0) {
 					select_new_active_agg = temp_aggregator->is_active;
-					// clear the aggregator
 					ad_clear_agg(temp_aggregator);
 					if (select_new_active_agg) {
 						pr_info("%s: Removing an active aggregator\n",
 							slave->bond->dev->name);
-						// select new active aggregator
+						/* select new active aggregator */
 						ad_agg_selection_logic(__get_first_agg(port));
 					}
 				}
@@ -2083,7 +2150,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		goto re_arm;
 
 	/* check if agg_select_timer timer after initialize is timed out */
-	if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
+	if (BOND_AD_INFO(bond).agg_select_timer &&
+	    !(--BOND_AD_INFO(bond).agg_select_timer)) {
 		slave = bond_first_slave_rcu(bond);
 		port = slave ? &(SLAVE_AD_INFO(slave).port) : NULL;
 
@@ -2145,7 +2213,8 @@ re_arm:
  * received frames (loopback). Since only the payload is given to this
  * function, it check for loopback.
  */
-static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u16 length)
+static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave,
+				  u16 length)
 {
 	struct port *port;
 	int ret = RX_HANDLER_ANOTHER;
@@ -2173,7 +2242,9 @@ static int bond_3ad_rx_indication(struct lacpdu *lacpdu, struct slave *slave, u1
 
 		case AD_TYPE_MARKER:
 			ret = RX_HANDLER_CONSUMED;
-			// No need to convert fields to Little Endian since we don't use the marker's fields.
+			/* No need to convert fields to Little Endian since we
+			 * don't use the marker's fields.
+			 */
 
 			switch (((struct bond_marker *)lacpdu)->tlv_type) {
 			case AD_MARKER_INFORMATION_SUBTYPE:
@@ -2317,10 +2388,13 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 	__release_state_machine_lock(port);
 }
 
-/*
- * set link state for bonding master: if we have an active
- * aggregator, we're up, if not, we're down.  Presumes that we cannot
- * have an active aggregator if there are no slaves with link up.
+/**
+ * bond_3ad_set_carrier - set link state for bonding master
+ * @bond - bonding structure
+ *
+ * if we have an active aggregator, we're up, if not, we're down.
+ * Presumes that we cannot have an active aggregator if there are
+ * no slaves with link up.
  *
  * This behavior complies with IEEE 802.3 section 43.3.9.
  *
@@ -2388,7 +2462,8 @@ int __bond_3ad_get_active_agg_info(struct bonding *bond,
 		ad_info->ports = aggregator->num_of_ports;
 		ad_info->actor_key = aggregator->actor_oper_aggregator_key;
 		ad_info->partner_key = aggregator->partner_oper_aggregator_key;
-		memcpy(ad_info->partner_system, aggregator->partner_system.mac_addr_value, ETH_ALEN);
+		memcpy(ad_info->partner_system,
+		       aggregator->partner_system.mac_addr_value, ETH_ALEN);
 		return 0;
 	}
 
@@ -2460,7 +2535,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	/* we couldn't find any suitable slave after the agg_no, so use the
-	 * first suitable found, if found. */
+	 * first suitable found, if found.
+	 */
 	if (first_ok_slave)
 		bond_dev_queue_xmit(bond, skb, first_ok_slave->dev);
 	else
@@ -2493,7 +2569,10 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
 	return ret;
 }
 
-/*
+/**
+ * bond_3ad_update_lacp_rate - change the lacp rate
+ * @bond - bonding struct
+ *
  * When modify lacp_rate parameter via sysfs,
  * update actor_oper_port_state of each port.
  *
-- 
1.8.4

^ permalink raw reply related

* Re: [GIT net-next] Open vSwitch
From: Jesse Gross @ 2014-01-08 15:10 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, David Miller
In-Reply-To: <52CD657F.7080806-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>

On Wed, Jan 8, 2014 at 9:49 AM, Zoltan Kiss <zoltan.kiss-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> wrote:
> Hi,
>
> I've tried the latest net-next on a Xenserver install with 1.9.3 userspace,
> and it seems this patch series broke it (at least after reverting that
> locally it works now). I haven't went too far yet checking what's the
> problem, but it seems the xenbrX device doesn't really receive too much of
> the traffic coming through the NIC. Is it expected?

What do you mean by doesn't receive too much traffic? What does it get?

^ permalink raw reply

* [PATCH net-next v2 5/5] tcp: metrics: Allow selective get/del of tcp-metrics based on src IP
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389193559-16756-1-git-send-email-christoph.paasch@uclouvain.be>

We want to be able to get/del tcp-metrics based on the src IP. This
patch adds the necessary parsing of the netlink attribute and if the
source address is set, it will match on this one too.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/ipv4/tcp_metrics.c | 48 ++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index e150f264c8e2..699a42faab9c 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -877,44 +877,66 @@ done:
 	return skb->len;
 }
 
-static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
-			 unsigned int *hash, int optional)
+static int __parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
+			   unsigned int *hash, int optional, int v4, int v6)
 {
 	struct nlattr *a;
 
-	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV4];
+	a = info->attrs[v4];
 	if (a) {
 		addr->family = AF_INET;
 		addr->addr.a4 = nla_get_be32(a);
-		*hash = (__force unsigned int) addr->addr.a4;
+		if (hash)
+			*hash = (__force unsigned int) addr->addr.a4;
 		return 0;
 	}
-	a = info->attrs[TCP_METRICS_ATTR_ADDR_IPV6];
+	a = info->attrs[v6];
 	if (a) {
 		if (nla_len(a) != sizeof(struct in6_addr))
 			return -EINVAL;
 		addr->family = AF_INET6;
 		memcpy(addr->addr.a6, nla_data(a), sizeof(addr->addr.a6));
-		*hash = ipv6_addr_hash((struct in6_addr *) addr->addr.a6);
+		if (hash)
+			*hash = ipv6_addr_hash((struct in6_addr *) addr->addr.a6);
 		return 0;
 	}
 	return optional ? 1 : -EAFNOSUPPORT;
 }
 
+static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
+			 unsigned int *hash, int optional)
+{
+	return __parse_nl_addr(info, addr, hash, optional,
+			       TCP_METRICS_ATTR_ADDR_IPV4,
+			       TCP_METRICS_ATTR_ADDR_IPV6);
+}
+
+static int parse_nl_saddr(struct genl_info *info, struct inetpeer_addr *addr)
+{
+	return __parse_nl_addr(info, addr, NULL, 0,
+			       TCP_METRICS_ATTR_SADDR_IPV4,
+			       TCP_METRICS_ATTR_SADDR_IPV6);
+}
+
 static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr daddr;
+	struct inetpeer_addr saddr, daddr;
 	unsigned int hash;
 	struct sk_buff *msg;
 	struct net *net = genl_info_net(info);
 	void *reply;
 	int ret;
+	bool src = true;
 
 	ret = parse_nl_addr(info, &daddr, &hash, 0);
 	if (ret < 0)
 		return ret;
 
+	ret = parse_nl_saddr(info, &saddr);
+	if (ret < 0)
+		src = false;
+
 	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
 	if (!msg)
 		return -ENOMEM;
@@ -929,7 +951,8 @@ static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_daddr, &daddr)) {
+		if (addr_same(&tm->tcpm_daddr, &daddr) &&
+		    (!src || addr_same(&tm->tcpm_saddr, &saddr))) {
 			ret = tcp_metrics_fill_info(msg, tm);
 			break;
 		}
@@ -984,23 +1007,28 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	struct tcpm_hash_bucket *hb;
 	struct tcp_metrics_block *tm, *tmlist = NULL;
 	struct tcp_metrics_block __rcu **pp;
-	struct inetpeer_addr daddr;
+	struct inetpeer_addr saddr, daddr;
 	unsigned int hash;
 	struct net *net = genl_info_net(info);
 	int ret;
+	bool src = true;
 
 	ret = parse_nl_addr(info, &daddr, &hash, 1);
 	if (ret < 0)
 		return ret;
 	if (ret > 0)
 		return tcp_metrics_flush_all(net);
+	ret = parse_nl_saddr(info, &saddr);
+	if (ret < 0)
+		src = false;
 
 	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
 	hb = net->ipv4.tcp_metrics_hash + hash;
 	pp = &hb->chain;
 	spin_lock_bh(&tcp_metrics_lock);
 	for (tm = deref_locked_genl(*pp); tm; tm = deref_locked_genl(*pp)) {
-		if (addr_same(&tm->tcpm_daddr, &daddr)) {
+		if (addr_same(&tm->tcpm_daddr, &daddr) &&
+		    (!src || addr_same(&tm->tcpm_saddr, &saddr))) {
 			*pp = tm->tcpm_next;
 			tm->tcpm_next = tmlist;
 			tmlist = tm;
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net-next v2 4/5] tcp: metrics: Delete all entries matching a certain destination
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389193559-16756-1-git-send-email-christoph.paasch@uclouvain.be>

As we now can have multiple entries per destination-IP, the "ip
tcp_metrics delete address ADDRESS" command deletes all of them.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/ipv4/tcp_metrics.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 199659f7a871..e150f264c8e2 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -982,7 +982,7 @@ static int tcp_metrics_flush_all(struct net *net)
 static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 {
 	struct tcpm_hash_bucket *hb;
-	struct tcp_metrics_block *tm;
+	struct tcp_metrics_block *tm, *tmlist = NULL;
 	struct tcp_metrics_block __rcu **pp;
 	struct inetpeer_addr daddr;
 	unsigned int hash;
@@ -999,17 +999,22 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	hb = net->ipv4.tcp_metrics_hash + hash;
 	pp = &hb->chain;
 	spin_lock_bh(&tcp_metrics_lock);
-	for (tm = deref_locked_genl(*pp); tm;
-	     pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) {
+	for (tm = deref_locked_genl(*pp); tm; tm = deref_locked_genl(*pp)) {
 		if (addr_same(&tm->tcpm_daddr, &daddr)) {
 			*pp = tm->tcpm_next;
-			break;
+			tm->tcpm_next = tmlist;
+			tmlist = tm;
+		} else {
+			pp = &tm->tcpm_next;
 		}
 	}
 	spin_unlock_bh(&tcp_metrics_lock);
-	if (!tm)
+	if (!tmlist)
 		return -ESRCH;
-	kfree_rcu(tm, rcu_head);
+	for (tm = tmlist; tm; tm = tmlist) {
+		tmlist = tm->tcpm_next;
+		kfree_rcu(tm, rcu_head);
+	}
 	return 0;
 }
 
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net-next v2 3/5] tcp: metrics: New netlink attribute for src IP and dumped in netlink reply
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389193559-16756-1-git-send-email-christoph.paasch@uclouvain.be>

This patch adds a new netlink attribute for the source-IP and appends it
to the netlink reply. Now, iproute2 can have access to the source-IP.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 include/uapi/linux/tcp_metrics.h | 2 ++
 net/ipv4/tcp_metrics.c           | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/include/uapi/linux/tcp_metrics.h b/include/uapi/linux/tcp_metrics.h
index cb5157b55f32..54a37b13f2c4 100644
--- a/include/uapi/linux/tcp_metrics.h
+++ b/include/uapi/linux/tcp_metrics.h
@@ -35,6 +35,8 @@ enum {
 	TCP_METRICS_ATTR_FOPEN_SYN_DROPS,	/* u16, count of drops */
 	TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,	/* msecs age */
 	TCP_METRICS_ATTR_FOPEN_COOKIE,		/* binary */
+	TCP_METRICS_ATTR_SADDR_IPV4,		/* u32 */
+	TCP_METRICS_ATTR_SADDR_IPV6,		/* binary */
 
 	__TCP_METRICS_ATTR_MAX,
 };
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index de32aa41a846..199659f7a871 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -745,11 +745,17 @@ static int tcp_metrics_fill_info(struct sk_buff *msg,
 		if (nla_put_be32(msg, TCP_METRICS_ATTR_ADDR_IPV4,
 				tm->tcpm_daddr.addr.a4) < 0)
 			goto nla_put_failure;
+		if (nla_put_be32(msg, TCP_METRICS_ATTR_SADDR_IPV4,
+				tm->tcpm_saddr.addr.a4) < 0)
+			goto nla_put_failure;
 		break;
 	case AF_INET6:
 		if (nla_put(msg, TCP_METRICS_ATTR_ADDR_IPV6, 16,
 			    tm->tcpm_daddr.addr.a6) < 0)
 			goto nla_put_failure;
+		if (nla_put(msg, TCP_METRICS_ATTR_SADDR_IPV6, 16,
+			    tm->tcpm_saddr.addr.a6) < 0)
+			goto nla_put_failure;
 		break;
 	default:
 		return -EAFNOSUPPORT;
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net-next v2 2/5] tcp: metrics: Add source-address to tcp-metrics
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389193559-16756-1-git-send-email-christoph.paasch@uclouvain.be>

We add the source-address to the tcp-metrics, so that different metrics
will be used per source/destination-pair. We use the destination-hash to
store the metric inside the hash-table. That way, deleting and dumping
via "ip tcp_metrics" is easy.

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/ipv4/tcp_metrics.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index f9b5f519a4ea..de32aa41a846 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -31,6 +31,7 @@ struct tcp_fastopen_metrics {
 
 struct tcp_metrics_block {
 	struct tcp_metrics_block __rcu	*tcpm_next;
+	struct inetpeer_addr		tcpm_saddr;
 	struct inetpeer_addr		tcpm_daddr;
 	unsigned long			tcpm_stamp;
 	u32				tcpm_ts;
@@ -131,6 +132,7 @@ static void tcpm_suck_dst(struct tcp_metrics_block *tm, struct dst_entry *dst,
 }
 
 static struct tcp_metrics_block *tcpm_new(struct dst_entry *dst,
+					  struct inetpeer_addr *saddr,
 					  struct inetpeer_addr *daddr,
 					  unsigned int hash,
 					  bool reclaim)
@@ -155,6 +157,7 @@ static struct tcp_metrics_block *tcpm_new(struct dst_entry *dst,
 		if (!tm)
 			goto out_unlock;
 	}
+	tm->tcpm_saddr = *saddr;
 	tm->tcpm_daddr = *daddr;
 
 	tcpm_suck_dst(tm, dst, true);
@@ -189,7 +192,8 @@ static struct tcp_metrics_block *tcp_get_encode(struct tcp_metrics_block *tm, in
 	return NULL;
 }
 
-static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *daddr,
+static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *saddr,
+						   const struct inetpeer_addr *daddr,
 						   struct net *net, unsigned int hash)
 {
 	struct tcp_metrics_block *tm;
@@ -197,7 +201,8 @@ static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *d
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_daddr, daddr))
+		if (addr_same(&tm->tcpm_saddr, saddr) &&
+		    addr_same(&tm->tcpm_daddr, daddr))
 			break;
 		depth++;
 	}
@@ -208,18 +213,21 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 						       struct dst_entry *dst)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr daddr;
+	struct inetpeer_addr saddr, daddr;
 	unsigned int hash;
 	struct net *net;
 
+	saddr.family = req->rsk_ops->family;
 	daddr.family = req->rsk_ops->family;
 	switch (daddr.family) {
 	case AF_INET:
+		saddr.addr.a4 = inet_rsk(req)->ir_loc_addr;
 		daddr.addr.a4 = inet_rsk(req)->ir_rmt_addr;
 		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
+		*(struct in6_addr *)saddr.addr.a6 = inet_rsk(req)->ir_v6_loc_addr;
 		*(struct in6_addr *)daddr.addr.a6 = inet_rsk(req)->ir_v6_rmt_addr;
 		hash = ipv6_addr_hash(&inet_rsk(req)->ir_v6_rmt_addr);
 		break;
@@ -233,7 +241,8 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_daddr, &daddr))
+		if (addr_same(&tm->tcpm_saddr, &saddr) &&
+		    addr_same(&tm->tcpm_daddr, &daddr))
 			break;
 	}
 	tcpm_check_stamp(tm, dst);
@@ -243,18 +252,21 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock *tw)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr daddr;
+	struct inetpeer_addr saddr, daddr;
 	unsigned int hash;
 	struct net *net;
 
+	saddr.family = tw->tw_family;
 	daddr.family = tw->tw_family;
 	switch (daddr.family) {
 	case AF_INET:
+		saddr.addr.a4 = tw->tw_rcv_saddr;
 		daddr.addr.a4 = tw->tw_daddr;
 		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
+		*(struct in6_addr *)saddr.addr.a6 = tw->tw_v6_rcv_saddr;
 		*(struct in6_addr *)daddr.addr.a6 = tw->tw_v6_daddr;
 		hash = ipv6_addr_hash(&tw->tw_v6_daddr);
 		break;
@@ -268,7 +280,8 @@ static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_daddr, &daddr))
+		if (addr_same(&tm->tcpm_saddr, &saddr) &&
+		    addr_same(&tm->tcpm_daddr, &daddr))
 			break;
 	}
 	return tm;
@@ -279,19 +292,22 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
 						 bool create)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr daddr;
+	struct inetpeer_addr saddr, daddr;
 	unsigned int hash;
 	struct net *net;
 	bool reclaim;
 
+	saddr.family = sk->sk_family;
 	daddr.family = sk->sk_family;
 	switch (daddr.family) {
 	case AF_INET:
+		saddr.addr.a4 = inet_sk(sk)->inet_saddr;
 		daddr.addr.a4 = inet_sk(sk)->inet_daddr;
 		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
+		*(struct in6_addr *)saddr.addr.a6 = sk->sk_v6_rcv_saddr;
 		*(struct in6_addr *)daddr.addr.a6 = sk->sk_v6_daddr;
 		hash = ipv6_addr_hash(&sk->sk_v6_daddr);
 		break;
@@ -303,14 +319,14 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
 	net = dev_net(dst->dev);
 	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
 
-	tm = __tcp_get_metrics(&daddr, net, hash);
+	tm = __tcp_get_metrics(&saddr, &daddr, net, hash);
 	reclaim = false;
 	if (tm == TCP_METRICS_RECLAIM_PTR) {
 		reclaim = true;
 		tm = NULL;
 	}
 	if (!tm && create)
-		tm = tcpm_new(dst, &daddr, hash, reclaim);
+		tm = tcpm_new(dst, &saddr, &daddr, hash, reclaim);
 	else
 		tcpm_check_stamp(tm, dst);
 
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net-next v2 1/5] tcp: metrics: rename tcpm_addr to tcpm_daddr
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov
In-Reply-To: <1389193559-16756-1-git-send-email-christoph.paasch@uclouvain.be>

As we will add also the source-address, we rename all accesses to the
tcp-metrics address to use "daddr".

Signed-off-by: Christoph Paasch <christoph.paasch@uclouvain.be>
---
 net/ipv4/tcp_metrics.c | 72 +++++++++++++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 06493736fbc8..f9b5f519a4ea 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -31,7 +31,7 @@ struct tcp_fastopen_metrics {
 
 struct tcp_metrics_block {
 	struct tcp_metrics_block __rcu	*tcpm_next;
-	struct inetpeer_addr		tcpm_addr;
+	struct inetpeer_addr		tcpm_daddr;
 	unsigned long			tcpm_stamp;
 	u32				tcpm_ts;
 	u32				tcpm_ts_stamp;
@@ -131,7 +131,7 @@ static void tcpm_suck_dst(struct tcp_metrics_block *tm, struct dst_entry *dst,
 }
 
 static struct tcp_metrics_block *tcpm_new(struct dst_entry *dst,
-					  struct inetpeer_addr *addr,
+					  struct inetpeer_addr *daddr,
 					  unsigned int hash,
 					  bool reclaim)
 {
@@ -155,7 +155,7 @@ static struct tcp_metrics_block *tcpm_new(struct dst_entry *dst,
 		if (!tm)
 			goto out_unlock;
 	}
-	tm->tcpm_addr = *addr;
+	tm->tcpm_daddr = *daddr;
 
 	tcpm_suck_dst(tm, dst, true);
 
@@ -189,7 +189,7 @@ static struct tcp_metrics_block *tcp_get_encode(struct tcp_metrics_block *tm, in
 	return NULL;
 }
 
-static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *addr,
+static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *daddr,
 						   struct net *net, unsigned int hash)
 {
 	struct tcp_metrics_block *tm;
@@ -197,7 +197,7 @@ static struct tcp_metrics_block *__tcp_get_metrics(const struct inetpeer_addr *a
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, addr))
+		if (addr_same(&tm->tcpm_daddr, daddr))
 			break;
 		depth++;
 	}
@@ -208,19 +208,19 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 						       struct dst_entry *dst)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	struct inetpeer_addr daddr;
 	unsigned int hash;
 	struct net *net;
 
-	addr.family = req->rsk_ops->family;
-	switch (addr.family) {
+	daddr.family = req->rsk_ops->family;
+	switch (daddr.family) {
 	case AF_INET:
-		addr.addr.a4 = inet_rsk(req)->ir_rmt_addr;
-		hash = (__force unsigned int) addr.addr.a4;
+		daddr.addr.a4 = inet_rsk(req)->ir_rmt_addr;
+		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
-		*(struct in6_addr *)addr.addr.a6 = inet_rsk(req)->ir_v6_rmt_addr;
+		*(struct in6_addr *)daddr.addr.a6 = inet_rsk(req)->ir_v6_rmt_addr;
 		hash = ipv6_addr_hash(&inet_rsk(req)->ir_v6_rmt_addr);
 		break;
 #endif
@@ -233,7 +233,7 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr))
+		if (addr_same(&tm->tcpm_daddr, &daddr))
 			break;
 	}
 	tcpm_check_stamp(tm, dst);
@@ -243,19 +243,19 @@ static struct tcp_metrics_block *__tcp_get_metrics_req(struct request_sock *req,
 static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock *tw)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	struct inetpeer_addr daddr;
 	unsigned int hash;
 	struct net *net;
 
-	addr.family = tw->tw_family;
-	switch (addr.family) {
+	daddr.family = tw->tw_family;
+	switch (daddr.family) {
 	case AF_INET:
-		addr.addr.a4 = tw->tw_daddr;
-		hash = (__force unsigned int) addr.addr.a4;
+		daddr.addr.a4 = tw->tw_daddr;
+		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
-		*(struct in6_addr *)addr.addr.a6 = tw->tw_v6_daddr;
+		*(struct in6_addr *)daddr.addr.a6 = tw->tw_v6_daddr;
 		hash = ipv6_addr_hash(&tw->tw_v6_daddr);
 		break;
 #endif
@@ -268,7 +268,7 @@ static struct tcp_metrics_block *__tcp_get_metrics_tw(struct inet_timewait_sock
 
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr))
+		if (addr_same(&tm->tcpm_daddr, &daddr))
 			break;
 	}
 	return tm;
@@ -279,20 +279,20 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
 						 bool create)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	struct inetpeer_addr daddr;
 	unsigned int hash;
 	struct net *net;
 	bool reclaim;
 
-	addr.family = sk->sk_family;
-	switch (addr.family) {
+	daddr.family = sk->sk_family;
+	switch (daddr.family) {
 	case AF_INET:
-		addr.addr.a4 = inet_sk(sk)->inet_daddr;
-		hash = (__force unsigned int) addr.addr.a4;
+		daddr.addr.a4 = inet_sk(sk)->inet_daddr;
+		hash = (__force unsigned int) daddr.addr.a4;
 		break;
 #if IS_ENABLED(CONFIG_IPV6)
 	case AF_INET6:
-		*(struct in6_addr *)addr.addr.a6 = sk->sk_v6_daddr;
+		*(struct in6_addr *)daddr.addr.a6 = sk->sk_v6_daddr;
 		hash = ipv6_addr_hash(&sk->sk_v6_daddr);
 		break;
 #endif
@@ -303,14 +303,14 @@ static struct tcp_metrics_block *tcp_get_metrics(struct sock *sk,
 	net = dev_net(dst->dev);
 	hash = hash_32(hash, net->ipv4.tcp_metrics_hash_log);
 
-	tm = __tcp_get_metrics(&addr, net, hash);
+	tm = __tcp_get_metrics(&daddr, net, hash);
 	reclaim = false;
 	if (tm == TCP_METRICS_RECLAIM_PTR) {
 		reclaim = true;
 		tm = NULL;
 	}
 	if (!tm && create)
-		tm = tcpm_new(dst, &addr, hash, reclaim);
+		tm = tcpm_new(dst, &daddr, hash, reclaim);
 	else
 		tcpm_check_stamp(tm, dst);
 
@@ -724,15 +724,15 @@ static int tcp_metrics_fill_info(struct sk_buff *msg,
 	struct nlattr *nest;
 	int i;
 
-	switch (tm->tcpm_addr.family) {
+	switch (tm->tcpm_daddr.family) {
 	case AF_INET:
 		if (nla_put_be32(msg, TCP_METRICS_ATTR_ADDR_IPV4,
-				tm->tcpm_addr.addr.a4) < 0)
+				tm->tcpm_daddr.addr.a4) < 0)
 			goto nla_put_failure;
 		break;
 	case AF_INET6:
 		if (nla_put(msg, TCP_METRICS_ATTR_ADDR_IPV6, 16,
-			    tm->tcpm_addr.addr.a6) < 0)
+			    tm->tcpm_daddr.addr.a6) < 0)
 			goto nla_put_failure;
 		break;
 	default:
@@ -882,14 +882,14 @@ static int parse_nl_addr(struct genl_info *info, struct inetpeer_addr *addr,
 static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 {
 	struct tcp_metrics_block *tm;
-	struct inetpeer_addr addr;
+	struct inetpeer_addr daddr;
 	unsigned int hash;
 	struct sk_buff *msg;
 	struct net *net = genl_info_net(info);
 	void *reply;
 	int ret;
 
-	ret = parse_nl_addr(info, &addr, &hash, 0);
+	ret = parse_nl_addr(info, &daddr, &hash, 0);
 	if (ret < 0)
 		return ret;
 
@@ -907,7 +907,7 @@ static int tcp_metrics_nl_cmd_get(struct sk_buff *skb, struct genl_info *info)
 	rcu_read_lock();
 	for (tm = rcu_dereference(net->ipv4.tcp_metrics_hash[hash].chain); tm;
 	     tm = rcu_dereference(tm->tcpm_next)) {
-		if (addr_same(&tm->tcpm_addr, &addr)) {
+		if (addr_same(&tm->tcpm_daddr, &daddr)) {
 			ret = tcp_metrics_fill_info(msg, tm);
 			break;
 		}
@@ -962,12 +962,12 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	struct tcpm_hash_bucket *hb;
 	struct tcp_metrics_block *tm;
 	struct tcp_metrics_block __rcu **pp;
-	struct inetpeer_addr addr;
+	struct inetpeer_addr daddr;
 	unsigned int hash;
 	struct net *net = genl_info_net(info);
 	int ret;
 
-	ret = parse_nl_addr(info, &addr, &hash, 1);
+	ret = parse_nl_addr(info, &daddr, &hash, 1);
 	if (ret < 0)
 		return ret;
 	if (ret > 0)
@@ -979,7 +979,7 @@ static int tcp_metrics_nl_cmd_del(struct sk_buff *skb, struct genl_info *info)
 	spin_lock_bh(&tcp_metrics_lock);
 	for (tm = deref_locked_genl(*pp); tm;
 	     pp = &tm->tcpm_next, tm = deref_locked_genl(*pp)) {
-		if (addr_same(&tm->tcpm_addr, &addr)) {
+		if (addr_same(&tm->tcpm_daddr, &daddr)) {
 			*pp = tm->tcpm_next;
 			break;
 		}
-- 
1.8.3.2

^ permalink raw reply related

* [PATCH net-next v2 0/5] Make tcp-metrics source-address aware
From: Christoph Paasch @ 2014-01-08 15:05 UTC (permalink / raw)
  To: netdev; +Cc: David Miller, Eric Dumazet, Yuchung Cheng, Julian Anastasov

Currently tcp-metrics only stores per-destination addresses. This brings
problems, when a host has multiple interfaces (e.g., a smartphone having
WiFi/3G):

For example, a host contacting a server over WiFi will store the tcp-metrics
per destination IP. If then the host contacts the same server over 3G, the
same tcp-metrics will be used, although the path-characteristics are completly
different (e.g., the ssthresh is probably not the same).

In case of TFO this is not a problem, as the server will provide us a new cookie
once he saw our SYN+DATA with an incorrect cookie.
It may be (in case of carrier-grade NAT), that we keep the same public IP but
have a different private IP. Thus, we better reuse the old cookie even if our
source-IP has changed. However, this scenario is probably very uncommon, as 
carriers try to provide the same src-IP to the clients behind their CGN.

Patches 1 + 2 add the source-IP to the tcp metrics.

Patches 3 to 5 modify the netlink-api to support the source-IP. From now on,
when using the command "ip tcp_metrics delete address ADDRESS" all entries
which match this destination IP will be deleted.

Today's iproute2 will complain when doing "ip tcp_metrics flush PREFIX" if
several entries are present for the same destination-IP but with different
source-IPs:

root@client:~/test# ip tcp_metrics
10.2.1.2 age 3.640sec rtt 16250us rttvar 15000us cwnd 10
10.2.1.2 age 4.030sec rtt 18750us rttvar 15000us cwnd 10
root@client:~/test# ip tcp_metrics flush 10.2.1.2/16
Failed to send flush request
: No such process


Follow-up patches will modify iproute2 to handle this correctly and allow
specifying the source-IP in the get/del commands.


v2: Added the patch that allows to selectively get/del of tcp-metrics based
    on src-IP and moved the patch that adds the new netlink attribute before
    the other patches.

Christoph Paasch (5):
  tcp: metrics: rename tcpm_addr to tcpm_daddr
  tcp: metrics: Add source-address to tcp-metrics
  tcp: metrics: New netlink attribute for src IP and dumped in netlink
    reply
  tcp: metrics: Delete all entries matching a certain destination
  tcp: metrics: Allow selective get/del of tcp-metrics based on src IP

 include/uapi/linux/tcp_metrics.h |   2 +
 net/ipv4/tcp_metrics.c           | 151 ++++++++++++++++++++++++++-------------
 2 files changed, 105 insertions(+), 48 deletions(-)

-- 
1.8.3.2

^ permalink raw reply

* Re: [ovs-dev] [GIT net-next] Open vSwitch
From: Zoltan Kiss @ 2014-01-08 14:49 UTC (permalink / raw)
  To: Jesse Gross, David Miller; +Cc: dev, netdev
In-Reply-To: <1389053776-62865-1-git-send-email-jesse@nicira.com>

Hi,

I've tried the latest net-next on a Xenserver install with 1.9.3 
userspace, and it seems this patch series broke it (at least after 
reverting that locally it works now). I haven't went too far yet 
checking what's the problem, but it seems the xenbrX device doesn't 
really receive too much of the traffic coming through the NIC. Is it 
expected?

Regards,

Zoli

On 07/01/14 00:15, Jesse Gross wrote:
> Open vSwitch changes for net-next/3.14. Highlights are:
>   * Performance improvements in the mechanism to get packets to userspace
>     using memory mapped netlink and skb zero copy where appropriate.
>   * Per-cpu flow stats in situations where flows are likely to be shared
>     across CPUs. Standard flow stats are used in other situations to save
>     memory and allocation time.
>   * A handful of code cleanups and rationalization.
>
> The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae:
>
>    Linux 3.13-rc1 (2013-11-22 11:30:55 -0800)
>
> are available in the git repository at:
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/jesse/openvswitch.git master
>
> for you to fetch changes up to 443cd88c8a31379e95326428bbbd40af25c1d440:
>
>    ovs: make functions local (2014-01-06 15:54:39 -0800)
>
> ----------------------------------------------------------------
> Andy Zhou (1):
>        openvswitch: Change ovs_flow_tbl_lookup_xx() APIs
>
> Ben Pfaff (2):
>        openvswitch: Correct comment.
>        openvswitch: Shrink sw_flow_mask by 8 bytes (64-bit) or 4 bytes (32-bit).
>
> Daniel Borkmann (1):
>        net: ovs: use kfree_rcu instead of rcu_free_{sw_flow_mask_cb,acts_callback}
>
> Jesse Gross (1):
>        openvswitch: Silence RCU lockdep checks from flow lookup.
>
> Pravin B Shelar (1):
>        openvswitch: Per cpu flow stats.
>
> Stephen Hemminger (1):
>        ovs: make functions local
>
> Thomas Graf (9):
>        genl: Add genlmsg_new_unicast() for unicast message allocation
>        netlink: Avoid netlink mmap alloc if msg size exceeds frame size
>        openvswitch: Enable memory mapped Netlink i/o
>        net: Export skb_zerocopy() to zerocopy from one skb to another
>        openvswitch: Allow user space to announce ability to accept unaligned Netlink messages
>        openvswitch: Drop user features if old user space attempted to create datapath
>        openvswitch: Pass datapath into userspace queue functions
>        openvswitch: Use skb_zerocopy() for upcall
>        openvswitch: Compute checksum in skb_gso_segment() if needed
>
> Wei Yongjun (1):
>        openvswitch: remove duplicated include from flow_table.c
>
>   include/linux/skbuff.h               |   3 +
>   include/net/genetlink.h              |   4 +
>   include/uapi/linux/openvswitch.h     |  14 ++-
>   net/core/skbuff.c                    |  85 +++++++++++++
>   net/netfilter/nfnetlink_queue_core.c |  59 +--------
>   net/netlink/af_netlink.c             |   4 +
>   net/netlink/genetlink.c              |  21 ++++
>   net/openvswitch/datapath.c           | 231 +++++++++++++++++++----------------
>   net/openvswitch/datapath.h           |   6 +-
>   net/openvswitch/flow.c               |  96 +++++++++++++--
>   net/openvswitch/flow.h               |  33 +++--
>   net/openvswitch/flow_netlink.c       |  66 ++++++++--
>   net/openvswitch/flow_netlink.h       |   1 +
>   net/openvswitch/flow_table.c         |  60 ++++++---
>   net/openvswitch/flow_table.h         |   6 +-
>   net/openvswitch/vport.c              |   6 +-
>   net/openvswitch/vport.h              |   1 -
>   17 files changed, 483 insertions(+), 213 deletions(-)
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>

^ permalink raw reply

* Re: [PATCH net-next v2] packet: doc: how to PACKET_MMAP with one packet socket for rx and tx
From: Daniel Borkmann @ 2014-01-08 14:48 UTC (permalink / raw)
  To: Norbert van Bolhuis; +Cc: netdev
In-Reply-To: <52CD64D4.5020005@aimvalley.nl>

On 01/08/2014 03:46 PM, Norbert van Bolhuis wrote:
>
> Describe how to use one AF_PACKET socket for rx and tx.
>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Norbert van Bolhuis <nvbolhuis@aimvalley.nl>

Acked-by: Daniel Borkmann <dborkman@redhat.com>

Thanks Norbert!

^ permalink raw reply

* [PATCH net-next v2] packet: doc: how to PACKET_MMAP with one packet socket for rx and tx
From: Norbert van Bolhuis @ 2014-01-08 14:46 UTC (permalink / raw)
  To: netdev; +Cc: Daniel Borkmann


Describe how to use one AF_PACKET socket for rx and tx.

Cc: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Norbert van Bolhuis <nvbolhuis@aimvalley.nl>
---
  Documentation/networking/packet_mmap.txt |   18 ++++++++++++++++++
  1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/Documentation/networking/packet_mmap.txt b/Documentation/networking/packet_mmap.txt
index 723bf3d..5566c20 100644
--- a/Documentation/networking/packet_mmap.txt
+++ b/Documentation/networking/packet_mmap.txt
@@ -98,6 +98,11 @@ by the kernel.
  The destruction of the socket and all associated resources
  is done by a simple call to close(fd).

+As without PACKET_MMAP, it is possible to use one socket for
+capture and transmission. This can be done by mapping the
+allocated RX and TX buffer ring with a single mmap() call. See
+"Mapping and use of the circular buffer (ring)".
+
  Next I will describe PACKET_MMAP settings and its constraints,
  also the mapping of the circular buffer in the user process and
  the use of this buffer.
@@ -414,6 +419,19 @@ tp_block_size/tp_frame_size frames there will be a gap between
  the frames. This is because a frame cannot be spawn across two
  blocks.

+To use one socket for capture and transmission, the mapping of both the RX
+and TX buffer ring has to be done with one call to mmap:
+
+    ...
+    setsockopt(fd, SOL_PACKET, PACKET_RX_RING, &foo, sizeof(foo));
+    setsockopt(fd, SOL_PACKET, PACKET_TX_RING, &bar, sizeof(bar));
+    ...
+    rx_ring = mmap(0, size * 2, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
+    tx_ring = rx_ring + size;
+
+RX must be the first as the kernel maps the TX ring memory right after
+the RX one.
+
  At the beginning of each frame there is an status field (see
  struct tpacket_hdr). If this field is 0 means that the frame is ready
  to be used for the kernel, If not, there is a frame the user can read

^ permalink raw reply related

* Re: [PATCH net-next v2 0/9] xen-netback: TX grant mapping with SKBTX_DEV_ZEROCOPY instead of copy
From: Zoltan Kiss @ 2014-01-08 14:44 UTC (permalink / raw)
  To: Wei Liu; +Cc: ian.campbell, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <20140108144324.GA6984@zion.uk.xensource.com>

On 08/01/14 14:43, Wei Liu wrote:
> You once mentioned that you have a trick to avoid touching TLB, is it in
> this series?
>
> (Haven't really looked at this series as I'm in today. Will have a
> closer look tonight. I'm just curious now.)
>
> Wei.
>
No, I'm currently working on that, it will be a separate series, as it 
also needs some Xen modifications which haven't reached upstream yet AFAIK.

Zoli

^ permalink raw reply

* Re: [PATCH net-next v2 0/9] xen-netback: TX grant mapping with SKBTX_DEV_ZEROCOPY instead of copy
From: Wei Liu @ 2014-01-08 14:43 UTC (permalink / raw)
  To: Zoltan Kiss
  Cc: ian.campbell, wei.liu2, xen-devel, netdev, linux-kernel,
	jonathan.davies
In-Reply-To: <1389139818-24458-1-git-send-email-zoltan.kiss@citrix.com>

You once mentioned that you have a trick to avoid touching TLB, is it in
this series?

(Haven't really looked at this series as I'm in today. Will have a
closer look tonight. I'm just curious now.)

Wei.

^ permalink raw reply

* [PATCH net] ipv6: add link-local, sit and loopback address with INFINITY_LIFE_TIME
From: Hannes Frederic Sowa @ 2014-01-08 14:43 UTC (permalink / raw)
  To: François-Xavier Le Bail; +Cc: netdev, davem, damien.wyart, yasushi.asano
In-Reply-To: <1389186135.88856.YahooMailBasic@web125506.mail.ne1.yahoo.com>

In the past the IFA_PERMANENT flag indicated, that the valid and preferred
lifetime where ignored. Since change fad8da3e085ddf ("ipv6 addrconf: fix
preferred lifetime state-changing behavior while valid_lft is infinity")
we honour at least the preferred lifetime on those addresses. As such
the valid lifetime gets recalculated and updated to 0.

If loopback address is added manually this problem does not occur.
Also if NetworkManager manages IPv6, those addresses will get added via
inet6_rtm_newaddr and thus will have a correct lifetime, too.

Reported-by: François-Xavier Le Bail <fx.lebail@yahoo.com>
Reported-by: Damien Wyart <damien.wyart@gmail.com>
Fixes: fad8da3e085ddf ("ipv6 addrconf: fix preferred lifetime state-changing behavior while valid_lft is infinity")
Cc: Yasushi Asano <yasushi.asano@jp.fujitsu.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
I hope there will be another pull request to Linus before 3.13 (I think
there will be an upcoming -rc8 this Sunday?).

 net/ipv6/addrconf.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31f75ea..01263a1 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2528,7 +2528,8 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
 	struct inet6_ifaddr *ifp;
 
 	ifp = ipv6_add_addr(idev, addr, NULL, plen,
-			    scope, IFA_F_PERMANENT, 0, 0);
+			    scope, IFA_F_PERMANENT,
+			    INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
 	if (!IS_ERR(ifp)) {
 		spin_lock_bh(&ifp->lock);
 		ifp->flags &= ~IFA_F_TENTATIVE;
@@ -2656,7 +2657,8 @@ static void addrconf_add_linklocal(struct inet6_dev *idev, const struct in6_addr
 #endif
 
 
-	ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags, 0, 0);
+	ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
+			    INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
 	if (!IS_ERR(ifp)) {
 		addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
 		addrconf_dad_start(ifp);
-- 
1.8.4.2

^ permalink raw reply related

* Re: [PATCH net 2/2] net: core: explicitly select a txq before doing l2 forwarding
From: Neil Horman @ 2014-01-08 14:40 UTC (permalink / raw)
  To: Jason Wang; +Cc: davem, netdev, linux-kernel, mst, John Fastabend, e1000-devel
In-Reply-To: <52CCC431.3060403@redhat.com>

On Wed, Jan 08, 2014 at 11:21:21AM +0800, Jason Wang wrote:
> On 01/07/2014 09:17 PM, Neil Horman wrote:
> > On Tue, Jan 07, 2014 at 11:42:24AM +0800, Jason Wang wrote:
> >> On 01/06/2014 08:42 PM, Neil Horman wrote:
> >>> On Mon, Jan 06, 2014 at 11:21:07AM +0800, Jason Wang wrote:
> >>>> Currently, the tx queue were selected implicitly in ndo_dfwd_start_xmit(). The
> >>>> will cause several issues:
> >>>>
> >>>> - NETIF_F_LLTX was forced for macvlan device in this case which lead extra lock
> >>>>   contention.
> >>>> - dev_hard_start_xmit() was called with NULL txq which bypasses the net device
> >>>>   watchdog
> >>>> - dev_hard_start_xmit() does not check txq everywhere which will lead a crash
> >>>>   when tso is disabled for lower device.
> >>>>
> >>>> Fix this by explicitly introducing a select queue method just for l2 forwarding
> >>>> offload (ndo_dfwd_select_queue), and introducing dfwd_direct_xmit() to do the
> >>>> queue selecting and transmitting for l2 forwarding.
> >>>>
> >>>> With this fixes, NETIF_F_LLTX could be preserved for macvlan and there's no need
> >>>> to check txq against NULL in dev_hard_start_xmit().
> >>>>
> >>>> In the future, it was also required for macvtap l2 forwarding support since it
> >>>> provides a necessary synchronization method.
> >>>>
> >>>> Cc: John Fastabend <john.r.fastabend@intel.com>
> >>>> Cc: Neil Horman <nhorman@tuxdriver.com>
> >>>> Cc: e1000-devel@lists.sourceforge.net
> >>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>> Instead of creating another operation here to do special queue selection, why
> >>> not just have ndo_dfwd_start_xmit include a pointer to a pointer in its argument
> >>> list, so it can pass the txq it used back to the caller (dev_hard_start_xmit)?
> >>> ndo_dfwd_start_xmit already knows which queue set to pick from (since their
> >>> reserved for the device doing the transmitting).  It seems more clear to me than
> >>> creating a new netdevice operation.  
> >> See commit 8ffab51b3dfc54876f145f15b351c41f3f703195 ("macvlan: lockless
> >> tx path"). The point is keep the tx path lockless to be efficient and
> >> simplicity for management. And macvtap multiqueue was also implemented
> >> with this assumption. The real contention should be done in the txq of
> >> lower device instead of macvlan itself. This is also needed for
> >> multiqueue macvtap.
> > Ok, I see how you're preserving LLTX here, and thats great, but it doesn't
> > really buy us anything that I can see.  If a macvlan is using hardware
> > acceleration, it needs to arbitrate access to that hardware.  Weather thats done
> > by locking the lowerdev's tx queue lock or by enforcing locking on the macvlan
> > itself is equivalent.  The decision to use dfwd hardware acceleration is made on
> > open, so its not like theres any traffic that can avoid the lock, as it all goes
> > through the hardware.  All I see that this has bought us is an extra net_device
> > method (which isn't a big deal, but not necessecary as I see it).
> 
> As I replied to patch 1/2, looking at the code itself again. The locking
> on the lowerdev's tx queue is really need since we need synchronize with
> other control path. Two examples are dev watchdog and ixgbe_down() both
> of which will try to hold tx lock to synchronize the with transmission.
> Without holding the lowerdev tx lock, we may have more serious issues.
> Also, it's a little strange for a net device has two modes. Future
> developers need to care about two different tx lock paths which is sub
> optimal.
> 

Ok, having looked at this for a few hours, I agree, locking in the lowerdev has
some definiate advantages in plugging the holes you've pointed out.

> For the issue of an extra net_device method,  if you don't like we can
> reuse the ndo_select_queue by also passing the accel_priv to that method.
I do, that actually simplifies things, since it lets us use the entire
dev_hard_start_xmit path unmodified, which gives us the locking your looking for
without having to create a new slimmed down variant of dev_hard_start_xmit.

Regards
Neil

^ permalink raw reply

* Re: single process receives own frames due to PACKET_MMAP
From: Norbert van Bolhuis @ 2014-01-08 14:36 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Jesper Dangaard Brouer, netdev, David Miller, uaca
In-Reply-To: <52CD5FAB.60801@redhat.com>

On 01/08/14 15:24, Daniel Borkmann wrote:
> On 01/08/2014 03:18 PM, Norbert van Bolhuis wrote:
>
>>>> Is there a way to use one PF_PACKET socket for both TX and RX and use PACKET_MMAP ?
>>>
>>> Yep:
>>>
>>> http://thread.gmane.org/gmane.linux.network/269129/focus=269188
>>>
>>> Feel free to make a patch and add this to Documentation/networking/packet_mmap.txt
>>> I think could be useful for others as well.
>>
>>
>> Good, it all works fine now, though performance is still not as good as I'd hoped.
>
> Does your use case allow for TPACKET_V3 or for using fanout ?
>

no, unfortunately not.

Though currently our performance is limited by application/system stuff, so
at the moment it doesn't really matter.

--
Norbert

^ permalink raw reply

* DSCP of ICMPv6 Echo Reply not copied from Echo Request
From: Simon Schneider @ 2014-01-08 14:34 UTC (permalink / raw)
  To: netdev


Hi,
with kernel version 2.6.33.32, I noticed for ping6 that the DSCP value of an Echo Request is not used for the corresponding Echo Reply.
 
With ICMPv4, this is the case and I think it makes sense.
 
This is not mandated by ICMP RFCs, but there are good reasons to use the same DSCP in both directions.
 
Is there a specific reason for this difference between v4 and v6?
 
Or was it even fixed in some newer kernel version?
 
best regards, Simon

^ 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