* Re: [PATCH net] bonding: fix arp requests sends with isolated routes
From: François Cachereul @ 2014-02-17 11:07 UTC (permalink / raw)
To: Veaceslav Falico; +Cc: Jay Vosburgh, Andy Gospodarek, netdev
In-Reply-To: <20140217093618.GA13038@redhat.com>
Le 17/02/2014 10:36, Veaceslav Falico a écrit :
> On Fri, Feb 14, 2014 at 04:59:23PM +0100, François Cachereul wrote:
>> Make arp_send_all() try to send arp packets through slave devices event
>> if no route to arp_ip_target is found. This is useful when the route
>> is in an isolated routing table with routing rule parameters like oif or
>> iif in which case ip_route_output() return an error.
>> Thus, the arp packet is send without vlan and with the bond ip address
>> as sender.
>
> I'm not sure I understand it completely, specifically I don't really
> understand the term "isolated routing table". Do you mean that it's an
> routing table different from local/main, which is enabled by
> CONFIG_IP_MULTIPLE_TABLES=y ? I think they should be all 'catched' by
> ip_route_output(), or am I missing something?
That what I meant, but like I said when the rule to lookup a table contains
iif or oif parameters (for example with the bond device value),
ip_route_output() can't lookup this table as flowi4_iif is set to
LOOPBACK_INDEX and we set flowi4_oif to 0 because we are searching for it.
>
> Anyway, with this fix bonding will send packets even if it doesn't find
> route AND src addr (bond_confirm_addr() can return 0 if bond doesn't have
> the required ip assigned), which isn't good at all.
We may had a test to avoid this case but that should also be done if the
route is found because bond_confirm_addr() could likewise return 0.
>
> If my assumption about the routing tables is correct and ip_route_output()
> doesn't find addition tables, we should at least try to fix it via scanning
> those tables.
ip_route_output() scan all tables for which the rule match parameters like
src network, dst network, oif or iif. In my case, it can't match iif or oif.
If we want to search all tables in any configurations we would have to bypass
route lookup process or at least the "rule match" part. I don't know if it's
a good idea.
Anyway, what I understand is that we are searching for a route only to find
a vlan to add to the output packet, but we don't care of the route. We just
want the packet to pass filtering switches to test the link.
I just propose a default configuration in order to succeed most time
(when we can't find any route) in a way to find a previous behavior in which
this use case worked.
> Sorry if I didn't understand you correctly...
No problem, you understand it well. Thanks for the reply.
>
>>
>> Signed-off-by: François CACHEREUL <f.cachereul@alphalink.fr>
>> ---
>> This previously worked, the problem was added in 2.6.35 with vlan 0
>> added by default when the module 8021q is loaded. Before that no route
>> lookup was done if the bond device did not have any vlan. The problem
>> now exists event if the module 8021q is not loaded.
>>
>> drivers/net/bonding/bond_main.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 8676649..300e5b8 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -2168,17 +2168,19 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
>> for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) {
>> pr_debug("basa: target %pI4\n", &targets[i]);
>>
>> + vlan_id = 0;
>> +
>> /* Find out through which dev should the packet go */
>> rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
>> RTO_ONLINK, 0);
>> if (IS_ERR(rt)) {
>> pr_debug("%s: no route to arp_ip_target %pI4\n",
>> bond->dev->name, &targets[i]);
>> - continue;
>> + /* no route found, trying with bond->dev */
>> + addr = bond_confirm_addr(bond->dev, targets[i], 0);
>> + goto rt_err_try;
>> }
>>
>> - vlan_id = 0;
>> -
>> /* bond device itself */
>> if (rt->dst.dev == bond->dev)
>> goto found;
>> @@ -2232,6 +2234,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
>> found:
>> addr = bond_confirm_addr(rt->dst.dev, targets[i], 0);
>> ip_rt_put(rt);
>> +rt_err_try:
>> bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
>> addr, vlan_id);
>> }
>> --
>> 1.7.10.4
>>
^ permalink raw reply
* Re: [Xen-devel] [PATCH V3 net-next 3/5] xen-netfront: Factor queue-specific data into queue struct.
From: Andrew Bennieston @ 2014-02-17 11:10 UTC (permalink / raw)
To: David Vrabel; +Cc: xen-devel, netdev, paul.durrant, wei.liu2, ian.campbell
In-Reply-To: <52FE5AC6.9000300@citrix.com>
On 14/02/14 18:04, David Vrabel wrote:
> On 14/02/14 17:35, Andrew J. Bennieston wrote:
>> From: "Andrew J. Bennieston" <andrew.bennieston@citrix.com>
>>
>> In preparation for multi-queue support in xen-netfront, move the
>> queue-specific data from struct netfront_info to struct netfront_queue,
>> and update the rest of the code to use this.
>>
>> Also adds loops over queues where appropriate, even though only one is
>> configured at this point, and uses alloc_etherdev_mq() and the
>> corresponding multi-queue netif wake/start/stop functions in preparation
>> for multiple active queues.
>>
>> Finally, implements a trivial queue selection function suitable for
>> ndo_select_queue, which simply returns 0, selecting the first (and
>> only) queue.
> [...]
>> --- a/drivers/net/xen-netfront.c
>> +++ b/drivers/net/xen-netfront.c
> [...]
>> @@ -2048,17 +2196,27 @@ static const struct xenbus_device_id netfront_ids[] = {
> [...]
>> + for (i = 0; i < info->num_queues; ++i) {
>> + queue = &info->queues[i];
>> + del_timer_sync(&queue->rx_refill_timer);
>> + }
>> +
>> + if (info->num_queues) {
>> + kfree(info->queues);
>> + info->queues = NULL;
>> + }
>> +
>> xennet_sysfs_delif(info->netdev);
>>
>> unregister_netdev(info->netdev);
>>
>> - del_timer_sync(&info->rx_refill_timer);
>> -
>
> This has reordered the del_timer_sync() to before the
> unregister_netdev() call.
>
> Can you be sure that the timer cannot be restarted after deleting it?
>
> David
>
Looking at the code, mod_timer() is called from
xennet_alloc_rx_buffers(), only. This, in turn, is called from
xennet_poll, which is the registered NAPI handler function. This should
not be called after a napi_disable(), which is done in xennet_close().
xennet_close() is called to stop the interface, which should be done
before the module is removed (unless I'm mistaken here). So this should
be safe.
That said, there is no reason that the queue cleanup has to happen
before the unregister_netdev() call. I'll move it to after that point,
just to be safe.
-Andrew
^ permalink raw reply
* [PATCH net] net: sctp: fix sctp_connectx abi for ia32 emulation/compat mode
From: Daniel Borkmann @ 2014-02-17 11:11 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp
SCTP's sctp_connectx() abi breaks for 64bit kernels compiled with 32bit
emulation (e.g. ia32 emulation or x86_x32). Due to internal usage of
'struct sctp_getaddrs_old' which includes a struct sockaddr pointer,
sizeof(param) check will always fail in kernel as the structure in
64bit kernel space is 4bytes larger than for user binaries compiled
in 32bit mode. Thus, applications making use of sctp_connectx() won't
be able to run under such circumstances.
Introduce a compat interface in the kernel to deal with such
situations by using a 'struct compat_sctp_getaddrs_old' structure
where user data is copied into it, and then sucessively transformed
into a 'struct sctp_getaddrs_old' structure with the help of
compat_ptr(). That fixes sctp_connectx() abi without any changes
needed in user space, and lets the SCTP test suite pass when compiled
in 32bit and run on 64bit kernels.
Fixes: f9c67811ebc0 ("sctp: Fix regression introduced by new sctp_connectx api")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/sctp/socket.c | 41 ++++++++++++++++++++++++++++++++---------
1 file changed, 32 insertions(+), 9 deletions(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 7075ac8..9145ffc 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -64,6 +64,7 @@
#include <linux/crypto.h>
#include <linux/slab.h>
#include <linux/file.h>
+#include <linux/compat.h>
#include <net/ip.h>
#include <net/icmp.h>
@@ -1368,11 +1369,19 @@ static int sctp_setsockopt_connectx(struct sock *sk,
/*
* New (hopefully final) interface for the API.
* We use the sctp_getaddrs_old structure so that use-space library
- * can avoid any unnecessary allocations. The only defferent part
+ * can avoid any unnecessary allocations. The only different part
* is that we store the actual length of the address buffer into the
- * addrs_num structure member. That way we can re-use the existing
+ * addrs_num structure member. That way we can re-use the existing
* code.
*/
+#ifdef CONFIG_COMPAT
+struct compat_sctp_getaddrs_old {
+ sctp_assoc_t assoc_id;
+ s32 addr_num;
+ compat_uptr_t addrs; /* struct sockaddr * */
+};
+#endif
+
static int sctp_getsockopt_connectx3(struct sock *sk, int len,
char __user *optval,
int __user *optlen)
@@ -1381,16 +1390,30 @@ static int sctp_getsockopt_connectx3(struct sock *sk, int len,
sctp_assoc_t assoc_id = 0;
int err = 0;
- if (len < sizeof(param))
- return -EINVAL;
+#ifdef CONFIG_COMPAT
+ if (is_compat_task()) {
+ struct compat_sctp_getaddrs_old param32;
- if (copy_from_user(¶m, optval, sizeof(param)))
- return -EFAULT;
+ if (len < sizeof(param32))
+ return -EINVAL;
+ if (copy_from_user(¶m32, optval, sizeof(param32)))
+ return -EFAULT;
- err = __sctp_setsockopt_connectx(sk,
- (struct sockaddr __user *)param.addrs,
- param.addr_num, &assoc_id);
+ param.assoc_id = param32.assoc_id;
+ param.addr_num = param32.addr_num;
+ param.addrs = compat_ptr(param32.addrs);
+ } else
+#endif
+ {
+ if (len < sizeof(param))
+ return -EINVAL;
+ if (copy_from_user(¶m, optval, sizeof(param)))
+ return -EFAULT;
+ }
+ err = __sctp_setsockopt_connectx(sk, (struct sockaddr __user *)
+ param.addrs, param.addr_num,
+ &assoc_id);
if (err == 0 || err == -EINPROGRESS) {
if (copy_to_user(optval, &assoc_id, sizeof(assoc_id)))
return -EFAULT;
--
1.7.11.7
^ permalink raw reply related
* [PATCH] ethtool: check the ethtool_ops is NULL in dev_ethtool
From: Wang Weidong @ 2014-02-17 11:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev
some drivers maybe not implement the ethtool_ops with only
set NULL. So when call the ethtool cmds will lead to a
'NULL pointer dereference'.
So add a checking in dev_ethtool.
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
---
net/core/ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 30071de..f418dcb 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1499,6 +1499,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
return -EPERM;
}
+
+ if (!dev->ethtool_ops)
+ return -EOPNOTSUPP;
if (dev->ethtool_ops->begin) {
rc = dev->ethtool_ops->begin(dev);
--
1.7.12
^ permalink raw reply related
* Re: [PATCH net] net: sctp: fix sctp_connectx abi for ia32 emulation/compat mode
From: Neil Horman @ 2014-02-17 12:17 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp
In-Reply-To: <1392635471-31528-1-git-send-email-dborkman@redhat.com>
On Mon, Feb 17, 2014 at 12:11:11PM +0100, Daniel Borkmann wrote:
> SCTP's sctp_connectx() abi breaks for 64bit kernels compiled with 32bit
> emulation (e.g. ia32 emulation or x86_x32). Due to internal usage of
> 'struct sctp_getaddrs_old' which includes a struct sockaddr pointer,
> sizeof(param) check will always fail in kernel as the structure in
> 64bit kernel space is 4bytes larger than for user binaries compiled
> in 32bit mode. Thus, applications making use of sctp_connectx() won't
> be able to run under such circumstances.
>
> Introduce a compat interface in the kernel to deal with such
> situations by using a 'struct compat_sctp_getaddrs_old' structure
> where user data is copied into it, and then sucessively transformed
> into a 'struct sctp_getaddrs_old' structure with the help of
> compat_ptr(). That fixes sctp_connectx() abi without any changes
> needed in user space, and lets the SCTP test suite pass when compiled
> in 32bit and run on 64bit kernels.
>
> Fixes: f9c67811ebc0 ("sctp: Fix regression introduced by new sctp_connectx api")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* [PATCH net-next v2] unix: add read side socket memory accounting for dgram sockets
From: Hannes Frederic Sowa @ 2014-02-17 13:03 UTC (permalink / raw)
To: netdev, yannick, eric.dumazet, davem, xiyou.wangcong, dan
In-Reply-To: <20140217035826.GG8634@order.stressinduktion.org>
The current situation:
We allocate memory with sock_alloc_send_pskb which accounts skb to the
sending socket's sk_wmem_alloc. The skb->destructor does a sock_wfree
as soon as the recipient retrieves the data and skb is destroyed.
The problem with this implementation is that an unconnected unix dgram
server could get blocked sending further data, if a client does not
retrieve its frames from its sk_receive_queue because sk_wmem_alloc is
under control of the client.
This patch does the following improvment:
We still allocate dgram packets with sock_alloc_send_pskb, which now
does normally not block if the socket has too many packets in flight.
While delivering the skb to the client in unix_dgram_sendmsg we check
for the recipients sockets rmem and block there if sk_rcvbuf is surpassed.
As the sender socket is not attached to the skb while retrieving it any
more, we must attach the unix dgram peers address to the skb directly,
this is done via a new field in unix_skb_parms.
Note, socket buffer limitation is already in place from the generic
socket layer. The client cannot request more than sysctl_rmem_max
(/proc/sys/net/core/rmem_max) bytes of receive buffer.
The (unix_peer(other) != sk) checks are removed as we now no longer block
in in sock_alloc_send_pskb if we try to send data to ourself, as send
buffer will always be free. To avoid DoS we need to bring the sender
to a stop trying to deliver skbs to its own socket receive queue here.
The same logic applies to the unix_dgram_poll change.
So, currently the following protections are in place to not get a victim
of a DoS:
* sk_receive_queue length limitation by sk_max_ack_backlog
* sk_rcvbuf length limiting on the receiving socket
* sending buffer limitations in case of too may concurrent send requests
With this patch it is possible to maybe relax the unix_recq_full check in
future or make the sk_max_ack_backlog configurable in future on a per-socket
basis.
Reported-by: Yannick Koehler <yannick@koehler.name>
CC: Yannick Koehler <yannick@koehler.name>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: David Miller <davem@davemloft.net>
Cc: Cong Wang <xiyou.wangcong@gmail.com>
Cc: Dan Ballard <dan@mindstab.net>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
Changelog v2:
In unix_wait_for_peer I messed up the condition of unix_other_writable to
determine if we should call schedule_timeout:
Instead of
sched = sched && (unix_recvq_full(other) ||
unix_other_writable(other));
it is now
sched = sched && (unix_recvq_full(other) ||
!unix_other_writable(other));
include/net/af_unix.h | 1 +
net/unix/af_unix.c | 79 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/include/net/af_unix.h b/include/net/af_unix.h
index a175ba4..52fbabd 100644
--- a/include/net/af_unix.h
+++ b/include/net/af_unix.h
@@ -36,6 +36,7 @@ struct unix_skb_parms {
u32 secid; /* Security ID */
#endif
u32 consumed;
+ struct unix_address *dgram_peer;
};
#define UNIXCB(skb) (*(struct unix_skb_parms *)&((skb)->cb))
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 29fc8be..103338e 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -187,6 +187,12 @@ static inline int unix_recvq_full(struct sock const *sk)
return skb_queue_len(&sk->sk_receive_queue) > sk->sk_max_ack_backlog;
}
+static bool unix_rmem_full(const struct sock *sk,
+ const struct sk_buff *skb)
+{
+ return sk_rmem_alloc_get(sk) + skb->truesize > sk->sk_rcvbuf;
+}
+
struct sock *unix_peer_get(struct sock *s)
{
struct sock *peer;
@@ -322,6 +328,11 @@ static inline int unix_writable(struct sock *sk)
return (atomic_read(&sk->sk_wmem_alloc) << 2) <= sk->sk_sndbuf;
}
+static int unix_other_writable(const struct sock *other)
+{
+ return (atomic_read(&other->sk_rmem_alloc) << 2) <= other->sk_rcvbuf;
+}
+
static void unix_write_space(struct sock *sk)
{
struct socket_wq *wq;
@@ -1048,8 +1059,13 @@ static long unix_wait_for_peer(struct sock *other, long timeo)
prepare_to_wait_exclusive(&u->peer_wait, &wait, TASK_INTERRUPTIBLE);
sched = !sock_flag(other, SOCK_DEAD) &&
- !(other->sk_shutdown & RCV_SHUTDOWN) &&
- unix_recvq_full(other);
+ !(other->sk_shutdown & RCV_SHUTDOWN);
+
+ if (other->sk_type == SOCK_STREAM)
+ sched = sched && unix_recvq_full(other);
+ else
+ sched = sched && (unix_recvq_full(other) ||
+ !unix_other_writable(other));
unix_state_unlock(other);
@@ -1367,9 +1383,33 @@ static void unix_destruct_scm(struct sk_buff *skb)
/* Alas, it calls VFS */
/* So fscking what? fput() had been SMP-safe since the last Summer */
scm_destroy(&scm);
+}
+
+static void unix_skb_destruct_r(struct sk_buff *skb)
+{
+ unix_destruct_scm(skb);
+ if (UNIXCB(skb).dgram_peer)
+ unix_release_addr(UNIXCB(skb).dgram_peer);
+ sock_rfree(skb);
+}
+
+static void unix_skb_destruct_w(struct sk_buff *skb)
+{
+ unix_destruct_scm(skb);
+ if (UNIXCB(skb).dgram_peer)
+ unix_release_addr(UNIXCB(skb).dgram_peer);
sock_wfree(skb);
}
+static void unix_skb_set_owner_r(struct sk_buff *skb, struct sock *sk,
+ struct sock *other)
+{
+ sock_wfree(skb);
+ skb->sk = other;
+ skb->destructor = unix_skb_destruct_r;
+ atomic_add(skb->truesize, &other->sk_rmem_alloc);
+}
+
#define MAX_RECURSION_LEVEL 4
static int unix_attach_fds(struct scm_cookie *scm, struct sk_buff *skb)
@@ -1417,7 +1457,6 @@ static int unix_scm_to_skb(struct scm_cookie *scm, struct sk_buff *skb, bool sen
if (scm->fp && send_fds)
err = unix_attach_fds(scm, skb);
- skb->destructor = unix_destruct_scm;
return err;
}
@@ -1504,6 +1543,12 @@ static int unix_dgram_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (skb == NULL)
goto out;
+ if (u->addr) {
+ UNIXCB(skb).dgram_peer = u->addr;
+ atomic_inc(&UNIXCB(skb).dgram_peer->refcnt);
+ }
+ skb->destructor = unix_skb_destruct_w;
+
err = unix_scm_to_skb(siocb->scm, skb, true);
if (err < 0)
goto out_free;
@@ -1579,7 +1624,7 @@ restart:
goto out_unlock;
}
- if (unix_peer(other) != sk && unix_recvq_full(other)) {
+ if (unix_recvq_full(other) || unix_rmem_full(other, skb)) {
if (!timeo) {
err = -EAGAIN;
goto out_unlock;
@@ -1597,6 +1642,7 @@ restart:
if (sock_flag(other, SOCK_RCVTSTAMP))
__net_timestamp(skb);
maybe_add_creds(skb, sock, other);
+ unix_skb_set_owner_r(skb, sk, other);
skb_queue_tail(&other->sk_receive_queue, skb);
if (max_level > unix_sk(other)->recursion_level)
unix_sk(other)->recursion_level = max_level;
@@ -1677,6 +1723,8 @@ static int unix_stream_sendmsg(struct kiocb *kiocb, struct socket *sock,
if (!skb)
goto out_err;
+ skb->destructor = unix_skb_destruct_w;
+
/* Only send the fds in the first buffer */
err = unix_scm_to_skb(siocb->scm, skb, !fds_sent);
if (err < 0) {
@@ -1760,13 +1808,12 @@ static int unix_seqpacket_recvmsg(struct kiocb *iocb, struct socket *sock,
return unix_dgram_recvmsg(iocb, sock, msg, size, flags);
}
-static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
+static void unix_copy_addr(struct msghdr *msg,
+ struct unix_address *ua)
{
- struct unix_sock *u = unix_sk(sk);
-
- if (u->addr) {
- msg->msg_namelen = u->addr->len;
- memcpy(msg->msg_name, u->addr->name, u->addr->len);
+ if (ua) {
+ msg->msg_namelen = ua->len;
+ memcpy(msg->msg_name, ua->name, ua->len);
}
}
@@ -1810,7 +1857,7 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
POLLOUT | POLLWRNORM | POLLWRBAND);
if (msg->msg_name)
- unix_copy_addr(msg, skb->sk);
+ unix_copy_addr(msg, UNIXCB(skb).dgram_peer);
if (size > skb->len - skip)
size = skb->len - skip;
@@ -2013,7 +2060,7 @@ again:
/* Copy address just once */
if (sunaddr) {
- unix_copy_addr(msg, skb->sk);
+ unix_copy_addr(msg, unix_sk(skb->sk)->addr);
sunaddr = NULL;
}
@@ -2237,11 +2284,9 @@ static unsigned int unix_dgram_poll(struct file *file, struct socket *sock,
writable = unix_writable(sk);
other = unix_peer_get(sk);
if (other) {
- if (unix_peer(other) != sk) {
- sock_poll_wait(file, &unix_sk(other)->peer_wait, wait);
- if (unix_recvq_full(other))
- writable = 0;
- }
+ sock_poll_wait(file, &unix_sk(other)->peer_wait, wait);
+ if (unix_recvq_full(other) || !unix_other_writable(other))
+ writable = 0;
sock_put(other);
}
--
1.8.5.3
^ permalink raw reply related
* Re: fe80::/64 route missing on GRE tunnels
From: Hannes Frederic Sowa @ 2014-02-17 13:04 UTC (permalink / raw)
To: Nicolas Dichtel; +Cc: netdev, itk-intern, sesse
In-Reply-To: <5301E59F.5090506@6wind.com>
On Mon, Feb 17, 2014 at 11:34:07AM +0100, Nicolas Dichtel wrote:
> Le 17/02/2014 11:17, Steinar H. Gunderson a écrit :
> >On Mon, Feb 17, 2014 at 10:59:38AM +0100, Nicolas Dichtel wrote:
> >>>It is added (ip -6 addr add fe80::1 dev foo), but it doesn't help
> >>>without the
> >>>/64 being present.
> >>With 'ip -6 addr add fe80::1/64 dev foo', the link local route will be
> >>added.
> >
> >Possible, but you're still breaking userspace. You could argue the tunnel
> >setup system in question should have added a /64 in the first place, but it
> >is deployed at a number of locations and changing all of them is not
> >trivial.
> I will propose a patch (instead of reverting the original one).
Thanks for looking into this, Nicolas. I agree it is not a very nice
situation.
^ permalink raw reply
* Re: [PATCH v2] ipv6: fix inconsistent prefix route handling
From: Hannes Frederic Sowa @ 2014-02-17 13:05 UTC (permalink / raw)
To: Steven Barth; +Cc: netdev
In-Reply-To: <5301AF26.7080102@openwrt.org>
On Mon, Feb 17, 2014 at 07:41:42AM +0100, Steven Barth wrote:
> Hannes Frederic Sowa wrote:
> >This could too easily clean up valid on-link information if the address
> >just happens to be in the same subnet. Would (ifp-flags &
> >(IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too?
>
> No, that doesn't do the trick unfortunately. Seems that the
> (non-permanent) address created by "ip" don't have IFA_F_TEMPORARY set.
> Any other good ideas?
We are talking about managed temp addresses, no? What ip command do you use?
Bye,
Hannes
^ permalink raw reply
* RFC: add init-regs for phy nodes
From: Ben Dooks @ 2014-02-17 13:08 UTC (permalink / raw)
To: netdev, devicetree; +Cc: linux-sh
I have run into an issue where the KSZ8041 does not get
initialised properly on the Renesas R8A7790 lager board.
This patch adds the ability to set arbitrary registers
on initialisation of a phy from informaiton in the fdt.
Feedback welcome.
^ permalink raw reply
* [PATCH] net: add init-regs for of_phy support
From: Ben Dooks @ 2014-02-17 13:08 UTC (permalink / raw)
To: netdev, devicetree; +Cc: linux-sh, Ben Dooks
In-Reply-To: <1392642484-19938-1-git-send-email-ben.dooks@codethink.co.uk>
Add new init-regs field for of_phy nodes and make sure these
get applied when the phy is configured.
This allows any phy node in an fdt to initialise registers
that may not be set as standard by the driver at initialisation
time, such as LED controls.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Documentation/devicetree/bindings/net/phy.txt | 12 ++++++
drivers/net/phy/phy_device.c | 59 ++++++++++++++++++++++++++-
2 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/phy.txt b/Documentation/devicetree/bindings/net/phy.txt
index 58307d0..48d8ded 100644
--- a/Documentation/devicetree/bindings/net/phy.txt
+++ b/Documentation/devicetree/bindings/net/phy.txt
@@ -20,6 +20,8 @@ Optional Properties:
assume clause 22. The compatible list may also contain other
elements.
- max-speed: Maximum PHY supported speed (10, 100, 1000...)
+- init-regs: Set of registers to modify at initialisation as a
+ a set of <register set clear>
Example:
@@ -29,3 +31,13 @@ ethernet-phy@0 {
interrupts = <35 1>;
reg = <0>;
};
+
+ethernet-phy@0 {
+ compatible = "ethernet-phy-ieee802.3-c22";
+ interrupt-parent = <40000>;
+ interrupts = <35 1>;
+ reg = <0>;
+
+ /* set KSZ8041 LED mode bits correctly */
+ init-reg = <0x1e 0x4000 0xc000>;
+};
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 82514e7..6741cdb 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -33,6 +33,7 @@
#include <linux/mdio.h>
#include <linux/io.h>
#include <linux/uaccess.h>
+#include <linux/of.h>
#include <asm/irq.h>
@@ -532,6 +533,57 @@ static int phy_poll_reset(struct phy_device *phydev)
return 0;
}
+#ifdef CONFIG_OF
+static int of_phy_configure(struct phy_device *phydev)
+{
+ struct device *dev = &phydev->dev;
+ struct device_node *of_node = dev->of_node;
+ struct property *prop;
+ const __be32 *ptr;
+ u32 reg, set, clear;
+ int len;
+ int val;
+
+ if (!of_node)
+ of_node = dev->parent->of_node;
+ if (!of_node)
+ return 0;
+
+ prop = of_find_property(of_node, "init-regs", &len);
+ if (prop) {
+ if (len % (sizeof(__be32) * 3)) {
+ dev_err(dev, "init-regs not multiple of 3 entries\n");
+ return -EINVAL;
+ }
+
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ while (ptr != NULL) {
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ ptr = of_prop_next_u32(prop, ptr, &set);
+ ptr = of_prop_next_u32(prop, ptr, &clear);
+
+ val = phy_read(phydev, reg);
+ if (val < 0) {
+ dev_err(dev, "failed to read %d\n", reg);
+ return val;
+ }
+
+ val &= ~clear;
+ val |= set;
+ phy_write(phydev, reg, val);
+
+ dev_info(dev, "set d to %04x\n", reg, val);
+
+ ptr = of_prop_next_u32(prop, ptr, ®);
+ }
+ }
+
+ return 0;
+}
+#else
+static inline int of_phy_configure(struct phy_device *phydev) { return 0; }
+#endif
+
int phy_init_hw(struct phy_device *phydev)
{
int ret;
@@ -551,7 +603,12 @@ int phy_init_hw(struct phy_device *phydev)
if (ret < 0)
return ret;
- return phydev->drv->config_init(phydev);
+ ret = phydev->drv->config_init(phydev);
+
+ if (ret == 0)
+ ret = of_phy_configure(phydev);
+
+ return ret;
}
EXPORT_SYMBOL(phy_init_hw);
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH v2] ipv6: fix inconsistent prefix route handling
From: Steven Barth @ 2014-02-17 13:13 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <20140217130557.GC22833@order.stressinduktion.org>
Ah sorry, maybe I was a bit unclear about this. With temporary addresses
I meant addresses created like this:
ip address add 2001:db8::1/64 dev eth0 valid_lft 1234 preferred_lft 1001
or the equivalent netlink-calls and with permanent addresses i was
referring to addresses created similarly but with an infinite lifetime
(e.g. through ip without giving valid_lft or preferred_lft arguments).
Hope that makes it clear.
Cheers,
Steven
On 17.02.2014 14:05, Hannes Frederic Sowa wrote:
> On Mon, Feb 17, 2014 at 07:41:42AM +0100, Steven Barth wrote:
>> Hannes Frederic Sowa wrote:
>>> This could too easily clean up valid on-link information if the address
>>> just happens to be in the same subnet. Would (ifp-flags &
>>> (IFA_F_PERMANENET|IFA_F_TEMPORARY) && !(...)) solve the problem, too?
>> No, that doesn't do the trick unfortunately. Seems that the
>> (non-permanent) address created by "ip" don't have IFA_F_TEMPORARY set.
>> Any other good ideas?
> We are talking about managed temp addresses, no? What ip command do you use?
>
> Bye,
>
> Hannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net] gre: add link local route when local addr is any
From: Nicolas Dichtel @ 2014-02-17 13:22 UTC (permalink / raw)
To: sesse; +Cc: hannes, itk-intern, davem, netdev, Nicolas Dichtel
In-Reply-To: <5301E59F.5090506@6wind.com>
This bug was reported by Steinar H. Gunderson and was introduced by commit
f7cb8886335d ("sit/gre6: don't try to add the same route two times").
root@morgental:~# ip tunnel add foo mode gre remote 1.2.3.4 ttl 64
root@morgental:~# ip link set foo up mtu 1468
root@morgental:~# ip -6 route show dev foo
fe80::/64 proto kernel metric 256
but after the above commit, no such route shows up.
There is no link local route because dev->dev_addr is 0 (because local ipv4
address is 0), hence no link local address is configured.
In this scenario, the link local address is added manually: 'ip -6 addr add
fe80::1 dev foo' and because prefix is /128, no link local route is added by the
kernel.
Even if the right things to do is to add the link local address with a /64
prefix, we need to restore the previous behavior to avoid breaking userpace.
Reported-by: Steinar H. Gunderson <sesse@samfundet.no>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
net/ipv6/addrconf.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ad235690684c..fdbfeca36d63 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2783,6 +2783,8 @@ static void addrconf_gre_config(struct net_device *dev)
ipv6_addr_set(&addr, htonl(0xFE800000), 0, 0, 0);
if (!ipv6_generate_eui64(addr.s6_addr + 8, dev))
addrconf_add_linklocal(idev, &addr);
+ else
+ addrconf_prefix_route(&addr, 64, dev, 0, 0);
}
#endif
--
1.8.5.4
^ permalink raw reply related
* Re: [PATCH v2 net-next 06/12] bonding: document the new _arp options for arp_validate
From: Veaceslav Falico @ 2014-02-17 13:36 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, Andy Gospodarek
In-Reply-To: <32051.1389991108@death.nxdomain>
On Fri, Jan 17, 2014 at 12:38:28PM -0800, Jay Vosburgh wrote:
>Veaceslav Falico <vfalico@redhat.com> wrote:
>
>>CC: Jay Vosburgh <fubar@us.ibm.com>
>>CC: Andy Gospodarek <andy@greyhouse.net>
>>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
>>---
>> Documentation/networking/bonding.txt | 34 ++++++++++++++++++++++++++++++----
>> 1 file changed, 30 insertions(+), 4 deletions(-)
>>
>>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>>index 3620690..a0c1bca2 100644
>>--- a/Documentation/networking/bonding.txt
>>+++ b/Documentation/networking/bonding.txt
>>@@ -279,19 +279,45 @@ arp_validate
>>
>> none or 0
>>
>>- No validation is performed. This is the default.
>>+ No validation is performed. This is the default. Any arriving
>>+ traffic (arp or non-arp) is considered a proof that the slave
>>+ is up.
>>
>> active or 1
>>
>>- Validation is performed only for the active slave.
>>+ Validation is performed only for the active slave. Only ARPs
>>+ that arrive from any arp_ip_target are considered legit. The
>>+ backup slave still does no validation (as if arp_validate=0).
>>
>> backup or 2
>>
>>- Validation is performed only for backup slaves.
>>+ Validation is performed only for backup slaves. Only ARPs
>>+ that arrive from any arp_ip_target are considered legit. The
>>+ active slave still has no validation (as if arp_validate=0).
>>
>> all or 3
>>
>>- Validation is performed for all slaves.
>>+ Validation is performed for all slaves. Only ARPs
>>+ that arrive from any arp_ip_target are considered legit.
>>+
>>+ arp or 4
>>+
>>+ Any arp packet is accepted as a proof that any slave is up,
>>+ but no IP-based validation is made.
>>+
>>+ active_arp or 5
>>+
>>+ Validation is performed only for the active slave. Only ARPs
>>+ that arrive from any arp_ip_target are considered legit. The
>>+ backup slave validates only arp packets, but doesn't check the
>>+ source (as if arp_validate=4).
>>+
>>+ backup_any or 6
>>+
>>+ Validation is performed only for backup slaves. Only ARPs
>>+ that arrive from any arp_ip_target are considered legit. The
>>+ active slave validates only arp packets, but doesn't check the
>>+ source (as if arp_validate=4).
Hi Jay,
Sorry for taking it so long - however I didn't manage to update/re-send the
patchset before the net-next closure, so sending it now. Few comments
below, though.
>
> I think that, for the last three options, saying that
>"validation is performed" is not quite right, since the next paragraph
>goes on to explain what "validation" is (that the incoming ARP came from
>us or was a response to ours), and these options don't really validate
>in that sense, but merely filter anything that's not an ARP.
Yes and no. Indeed, validation is not performed if we use the "filter_all"
option (filter both slaves, don't do any validation). However it's
performed for both filter_active/backup - as in "Validate arp packets on
active/backup, and filter only on the other one".
However your text is really awesome in explaining what is
validating/filtering, so I've included it in the patch with minor
corrections, see below.
>
> There'a a sentence with a similar problem further down: "Use of
>the arp_validate option can resolve this, as the ARP monitor will only
>consider ARP requests and replies associated with its own instance of
>bonding." For the three new options, this sentence is not accurate.
>
> I think I'd rework this whole block something like the following
>(this is a diff against your patched version). I'm calling the two
>separate things "validation" and "filtering," since the wording you used
>kind of combined things into two styles of validation; I think it's
>clearer to make them discrete things.
>
> This would also necessitate change the option tag names; I also
>put the "filter" ones into the same order as the "validate" ones
>(active, backup, then all).
I've left the original order, because the "filter" is, actually, bit 4, so
if we want to filter AND validate, we set FITLER | VALIDATE_{ACTIVE,BACKUP},
and this way we get 5 or 6. I've also changed the filter_all to just
"filter", to make it more clear what's happening. Here are the only lines
changed on top of your patch:
filter or 4
Filtering is applied to all slaves. No validation is
performed.
filter_active or 5
Filtering is applied to all slaves, validation is performed
only for the active slave.
filter_backup or 6
Filtering is applied to all slaves, validation is performed
only for backup slaves.
This way we can set any pair of filtering/validation:
No validation + no filtering? none
Validation + no filtering? acitve/backup
No validation + filtering? filter
Validation + filtering? filter_active/backup
I'll send v3 in a few minutes, so that it'll be easier to review (it's also
rebased on top of latest net-next).
Thanks a lot, and sorry for the delay.
>
> Comments?
>
>diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>index a0c1bca2..5fd6a6a 100644
>--- a/Documentation/networking/bonding.txt
>+++ b/Documentation/networking/bonding.txt
>@@ -270,80 +270,87 @@ arp_ip_target
> arp_validate
>
> Specifies whether or not ARP probes and replies should be
>- validated in any mode that supports arp monitoring. This causes
>- the ARP monitor to examine the incoming ARP requests and replies,
>- and only consider a slave to be up if it is receiving the
>- appropriate ARP traffic.
>-
>+ validated in any mode that supports arp monitoring, or whether
>+ non-ARP traffic should be filtered (disregarded) for link
>+ monitoring purposes.
>+
> Possible values are:
>
> none or 0
>
>- No validation is performed. This is the default. Any arriving
>- traffic (arp or non-arp) is considered a proof that the slave
>- is up.
>+ No validation or filtering is performed.
>
> active or 1
>
>- Validation is performed only for the active slave. Only ARPs
>- that arrive from any arp_ip_target are considered legit. The
>- backup slave still does no validation (as if arp_validate=0).
>+ Validation is performed only for the active slave.
>
> backup or 2
>
>- Validation is performed only for backup slaves. Only ARPs
>- that arrive from any arp_ip_target are considered legit. The
>- active slave still has no validation (as if arp_validate=0).
>+ Validation is performed only for backup slaves.
>
> all or 3
>
>- Validation is performed for all slaves. Only ARPs
>- that arrive from any arp_ip_target are considered legit.
>-
>- arp or 4
>-
>- Any arp packet is accepted as a proof that any slave is up,
>- but no IP-based validation is made.
>-
>- active_arp or 5
>-
>- Validation is performed only for the active slave. Only ARPs
>- that arrive from any arp_ip_target are considered legit. The
>- backup slave validates only arp packets, but doesn't check the
>- source (as if arp_validate=4).
>-
>- backup_any or 6
>-
>- Validation is performed only for backup slaves. Only ARPs
>- that arrive from any arp_ip_target are considered legit. The
>- active slave validates only arp packets, but doesn't check the
>- source (as if arp_validate=4).
>-
>- For the active slave, the validation checks ARP replies to
>- confirm that they were generated by an arp_ip_target. Since
>- backup slaves do not typically receive these replies, the
>- validation performed for backup slaves is on the ARP request
>- sent out via the active slave. It is possible that some
>- switch or network configurations may result in situations
>- wherein the backup slaves do not receive the ARP requests; in
>- such a situation, validation of backup slaves must be
>- disabled.
>-
>- The validation of ARP requests on backup slaves is mainly
>- helping bonding to decide which slaves are more likely to
>- work in case of the active slave failure, it doesn't really
>- guarantee that the backup slave will work if it's selected
>- as the next active slave.
>-
>- This option is useful in network configurations in which
>- multiple bonding hosts are concurrently issuing ARPs to one or
>- more targets beyond a common switch. Should the link between
>- the switch and target fail (but not the switch itself), the
>- probe traffic generated by the multiple bonding instances will
>- fool the standard ARP monitor into considering the links as
>- still up. Use of the arp_validate option can resolve this, as
>- the ARP monitor will only consider ARP requests and replies
>- associated with its own instance of bonding.
>+ Validation is performed for all slaves.
>+
>+ filter_active or 4
>+
>+ Filtering is applied to the active slave only.
>+
>+ filter_backup or 5
>+
>+ Filtering is applied to the backup slave(s) only.
>+
>+ filter_all or 6
>+
>+ Filtering is applied to all slaves.
>+
>+ Validation:
>+
>+ Enabling validation causes the ARP monitor to examine the incoming
>+ ARP requests and replies, and only consider a slave to be up if it
>+ is receiving the appropriate ARP traffic.
>+
>+ For an active slave, the validation checks ARP replies to confirm
>+ that they were generated by an arp_ip_target. Since backup slaves
>+ do not typically receive these replies, the validation performed
>+ for backup slaves is on the broadcast ARP request sent out via the
>+ active slave. It is possible that some switch or network
>+ configurations may result in situations wherein the backup slaves
>+ do not receive the ARP requests; in such a situation, validation
>+ of backup slaves must be disabled.
>+
>+ The validation of ARP requests on backup slaves is mainly helping
>+ bonding to decide which slaves are more likely to work in case of
>+ the active slave failure, it doesn't really guarantee that the
>+ backup slave will work if it's selected as the next active slave.
>+
>+ Validation is useful in network configurations in which multiple
>+ bonding hosts are concurrently issuing ARPs to one or more targets
>+ beyond a common switch. Should the link between the switch and
>+ target fail (but not the switch itself), the probe traffic
>+ generated by the multiple bonding instances will fool the standard
>+ ARP monitor into considering the links as still up. Use of
>+ validation can resolve this, as the ARP monitor will only consider
>+ ARP requests and replies associated with its own instance of
>+ bonding.
>+
>+ Filtering:
>+
>+ Enabling filtering causes the ARP monitor to only use incoming ARP
>+ packets for link availability purposes. Arriving packets that are
>+ not ARPs are delivered normally, but do not count when determining
>+ if a slave is available.
>+
>+ Filtering operates by only considering the reception of ARP
>+ packets (any ARP packet, regardless of source or destination) when
>+ determining if a slave has received traffic for link availability
>+ purposes.
>+
>+ Filtering is useful in network configurations in which significant
>+ levels of third party broadcast traffic would fool the standard
>+ ARP monitor into considering the links as still up. Use of
>+ filtering can resolve this, as only ARP traffic is considered for
>+ link availability purposes.
>
> This option was added in bonding version 3.1.0.
>
>
>
>---
> -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
>
^ permalink raw reply
* [PATCH v3 net-next 0/12] bonding: add an option to rely on unvalidated arp packets
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev
Cc: Rob Landley, Jay Vosburgh, Andy Gospodarek, David S. Miller,
dingtianhong, Nikolay Aleksandrov, Neil Horman, Cong Wang,
linux-doc, Veaceslav Falico
Hi,
v2 -> v3:
Per Jay's advise, use the 'filter' keyword instead of 'arp' one, and use
his text for documentation. Also, rebase on the latest net-next. Sorry for
the delay, didn't manage to send it before net-next was closed.
v1 -> v2:
Don't remove the 'all traffic' functionality - rather, add new arp_validate
options to specify that we want *only* unvalidated arps.
Currently, if arp_validate is off (0), slave_last_rx() returns the
slave->dev->last_rx, which is always updated on *any* packet received by
slave, and not only arps. This means that, if the validation of arps is
off, we're treating *any* incoming packet as a proof of slave being up, and
not only arps.
This might seem logical at the first glance, however it can cause a lot of
troubles and false-positives, one example would be:
The arp_ip_target is NOT accessible, however someone in the broadcast domain
spams with any broadcast traffic. This way bonding will be tricked that the
slave is still up (as in - can access arp_ip_target), while it's not.
The net_device->last_rx is already used in a lot of drivers (even though the
comment states to NOT do it :)), and it's also ugly to modify it from bonding.
However, some loadbalance setups might rely on the fact that even non-arp
traffic is a sign of slave being up - and we definitely can't break anyones
config - so an extension to arp_validate is needed.
So, to fix this, add an option for the user to specify if he wants to
filter out non-arp traffic on unvalidated slaves, remove the last_rx from
bonding, *always* call bond_arp_rcv() in slave's rx_handler (which is
bond_handle_frame), and if we spot an arp there with this option on - update
the slave->last_arp_rx - and use it instead of net_device->last_rx. Finally,
rename last_arp_rx to last_rx to reflect the changes.
Also rename slave->jiffies to ->last_link_up, to reflect better its
meaning, add the new option's documentation and update the arp_validate one
to be a bit more descriptive.
CC: Rob Landley <rob@landley.net>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
CC: dingtianhong <dingtianhong@huawei.com>
CC: Nikolay Aleksandrov <nikolay@redhat.com>
CC: Neil Horman <nhorman@tuxdriver.com>
CC: Cong Wang <amwang@redhat.com>
CC: linux-doc@vger.kernel.org
CC: netdev@vger.kernel.org
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Documentation/networking/bonding.txt | 96 +++++++++++++++++++++++++-----------
drivers/net/bonding/bond_main.c | 56 +++++++++------------
drivers/net/bonding/bond_options.c | 16 +++---
drivers/net/bonding/bonding.h | 26 ++++++----
include/linux/netdevice.h | 8 +--
5 files changed, 117 insertions(+), 85 deletions(-)
^ permalink raw reply
* [PATCH v3 net-next 01/12] bonding: remove bond->lock from bond_arp_rcv
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
We're always called with rcu_read_lock() held (bond_arp_rcv() is only
called from bond_handle_frame(), which is rx_handler and always called
under rcu from __netif_receive_skb_core() ).
The slave active/passive and/or bonding params can change in-flight, however
we don't really care about that - we only modify the last time packet was
received, which is harmless.
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_main.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3bce855..3c50bec 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2260,8 +2260,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
return RX_HANDLER_ANOTHER;
- read_lock(&bond->lock);
-
if (!slave_do_arp_validate(bond, slave))
goto out_unlock;
@@ -2318,7 +2316,6 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
bond_validate_arp(bond, slave, tip, sip);
out_unlock:
- read_unlock(&bond->lock);
if (arp != (struct arphdr *)skb->data)
kfree(arp);
return RX_HANDLER_ANOTHER;
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 02/12] bonding: permit using arp_validate with non-ab modes
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Currently it's diabled because it's sometimes hard, in typical configs, to
make it work - because of the nature how the loadbalance modes work - as
it's hard to deliver valid arp replies to correct slaves by the switch.
However we still can use arp_validation in loadbalance in several other
configs, per example with arp_validate == 2 for backup with one broadcast
domain, without the switch(es) doing any balancing - this way we'd be (a
bit more) sure that the slave is up.
So, enable it to let users decide which one works/suits them best.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
Documentation/networking/bonding.txt | 6 +++---
drivers/net/bonding/bond_main.c | 4 ----
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 5cdb229..96b4ad8 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -270,9 +270,9 @@ arp_ip_target
arp_validate
Specifies whether or not ARP probes and replies should be
- validated in the active-backup mode. This causes the ARP
- monitor to examine the incoming ARP requests and replies, and
- only consider a slave to be up if it is receiving the
+ validated in any mode that supports arp monitoring. This causes
+ the ARP monitor to examine the incoming ARP requests and replies,
+ and only consider a slave to be up if it is receiving the
appropriate ARP traffic.
Possible values are:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3c50bec..91c0248 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4183,10 +4183,6 @@ static int bond_check_params(struct bond_params *params)
}
if (arp_validate) {
- if (bond_mode != BOND_MODE_ACTIVEBACKUP) {
- pr_err("arp_validate only supported in active-backup mode\n");
- return -EINVAL;
- }
if (!arp_interval) {
pr_err("arp_validate requires arp_interval\n");
return -EINVAL;
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 04/12] bonding: always set recv_probe to bond_arp_rcv in arp monitor
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Currently we only set bond_arp_rcv() if we're using arp_validate, however
this makes us skip updating last_arp_rx if we're not validating incoming
ARPs - thus, if arp_validate is off, last_arp_rx will never be updated.
Fix this by always setting up recv_probe = bond_arp_rcv, even if we're not
using arp_validate.
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_main.c | 3 +--
drivers/net/bonding/bond_options.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7747cc5..257ee7f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3059,8 +3059,7 @@ static int bond_open(struct net_device *bond_dev)
if (bond->params.arp_interval) { /* arp interval, in milliseconds. */
queue_delayed_work(bond->wq, &bond->arp_work, 0);
- if (bond->params.arp_validate)
- bond->recv_probe = bond_arp_rcv;
+ bond->recv_probe = bond_arp_rcv;
}
if (bond->params.mode == BOND_MODE_8023AD) {
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index f3eb44d..cda686d 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -809,8 +809,7 @@ int bond_option_arp_interval_set(struct bonding *bond,
cancel_delayed_work_sync(&bond->arp_work);
} else {
/* arp_validate can be set only in active-backup mode */
- if (bond->params.arp_validate)
- bond->recv_probe = bond_arp_rcv;
+ bond->recv_probe = bond_arp_rcv;
cancel_delayed_work_sync(&bond->mii_work);
queue_delayed_work(bond->wq, &bond->arp_work, 0);
}
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 03/12] bonding: always update last_arp_rx on packet recieve
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Currently we're updating the last_arp_rx only when we've validate the
packet, however afterwards we use it as 'ANY last packet received', but not
only validated ARPs.
Fix this by updating it in case of any packet received. It won't break the
arp_validation=0 because we, anyway, return the correct slave->dev->last_rx in
slave_last_rx().
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_main.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 91c0248..7747cc5 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2257,6 +2257,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
__be32 sip, tip;
int alen;
+ slave->last_arp_rx = jiffies;
+
if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
return RX_HANDLER_ANOTHER;
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 05/12] bonding: extend arp_validate to be able to receive unvalidated arp-only traffic
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Currently we can either receive any traffic as a proff of slave being up,
or only *validated* arp traffic (i.e. with src/dst ip checked).
Add an option to be able to specify if we want to receive non-validated arp
traffic only.
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_options.c | 13 ++++++++-----
drivers/net/bonding/bonding.h | 11 +++++++++++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
index cda686d..beeec4c 100644
--- a/drivers/net/bonding/bond_options.c
+++ b/drivers/net/bonding/bond_options.c
@@ -47,11 +47,14 @@ static struct bond_opt_value bond_xmit_hashtype_tbl[] = {
};
static struct bond_opt_value bond_arp_validate_tbl[] = {
- { "none", BOND_ARP_VALIDATE_NONE, BOND_VALFLAG_DEFAULT},
- { "active", BOND_ARP_VALIDATE_ACTIVE, 0},
- { "backup", BOND_ARP_VALIDATE_BACKUP, 0},
- { "all", BOND_ARP_VALIDATE_ALL, 0},
- { NULL, -1, 0},
+ { "none", BOND_ARP_VALIDATE_NONE, BOND_VALFLAG_DEFAULT},
+ { "active", BOND_ARP_VALIDATE_ACTIVE, 0},
+ { "backup", BOND_ARP_VALIDATE_BACKUP, 0},
+ { "all", BOND_ARP_VALIDATE_ALL, 0},
+ { "filter", BOND_ARP_FILTER, 0},
+ { "filter_active", BOND_ARP_FILTER_ACTIVE, 0},
+ { "filter_backup", BOND_ARP_FILTER_BACKUP, 0},
+ { NULL, -1, 0},
};
static struct bond_opt_value bond_arp_all_targets_tbl[] = {
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 86ccfb9..ab2e651 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -342,6 +342,11 @@ static inline bool bond_is_active_slave(struct slave *slave)
#define BOND_ARP_VALIDATE_BACKUP (1 << BOND_STATE_BACKUP)
#define BOND_ARP_VALIDATE_ALL (BOND_ARP_VALIDATE_ACTIVE | \
BOND_ARP_VALIDATE_BACKUP)
+#define BOND_ARP_FILTER (BOND_ARP_VALIDATE_ALL + 1)
+#define BOND_ARP_FILTER_ACTIVE (BOND_ARP_VALIDATE_ACTIVE | \
+ BOND_ARP_FILTER)
+#define BOND_ARP_FILTER_BACKUP (BOND_ARP_VALIDATE_BACKUP | \
+ BOND_ARP_FILTER)
static inline int slave_do_arp_validate(struct bonding *bond,
struct slave *slave)
@@ -349,6 +354,12 @@ static inline int slave_do_arp_validate(struct bonding *bond,
return bond->params.arp_validate & (1 << bond_slave_state(slave));
}
+static inline int slave_do_arp_validate_only(struct bonding *bond,
+ struct slave *slave)
+{
+ return bond->params.arp_validate & BOND_ARP_FILTER;
+}
+
/* Get the oldest arp which we've received on this slave for bond's
* arp_targets.
*/
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 06/12] bonding: document the new _arp options for arp_validate
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-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>
---
Documentation/networking/bonding.txt | 96 +++++++++++++++++++++++++-----------
1 file changed, 66 insertions(+), 30 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 96b4ad8..a383c00 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -270,16 +270,15 @@ arp_ip_target
arp_validate
Specifies whether or not ARP probes and replies should be
- validated in any mode that supports arp monitoring. This causes
- the ARP monitor to examine the incoming ARP requests and replies,
- and only consider a slave to be up if it is receiving the
- appropriate ARP traffic.
+ validated in any mode that supports arp monitoring, or whether
+ non-ARP traffic should be filtered (disregarded) for link
+ monitoring purposes.
Possible values are:
none or 0
- No validation is performed. This is the default.
+ No validation or filtering is performed.
active or 1
@@ -293,31 +292,68 @@ arp_validate
Validation is performed for all slaves.
- For the active slave, the validation checks ARP replies to
- confirm that they were generated by an arp_ip_target. Since
- backup slaves do not typically receive these replies, the
- validation performed for backup slaves is on the ARP request
- sent out via the active slave. It is possible that some
- switch or network configurations may result in situations
- wherein the backup slaves do not receive the ARP requests; in
- such a situation, validation of backup slaves must be
- disabled.
-
- The validation of ARP requests on backup slaves is mainly
- helping bonding to decide which slaves are more likely to
- work in case of the active slave failure, it doesn't really
- guarantee that the backup slave will work if it's selected
- as the next active slave.
-
- This option is useful in network configurations in which
- multiple bonding hosts are concurrently issuing ARPs to one or
- more targets beyond a common switch. Should the link between
- the switch and target fail (but not the switch itself), the
- probe traffic generated by the multiple bonding instances will
- fool the standard ARP monitor into considering the links as
- still up. Use of the arp_validate option can resolve this, as
- the ARP monitor will only consider ARP requests and replies
- associated with its own instance of bonding.
+ filter or 4
+
+ Filtering is applied to all slaves. No validation is
+ performed.
+
+ filter_active or 5
+
+ Filtering is applied to all slaves, validation is performed
+ only for the active slave.
+
+ filter_backup or 6
+
+ Filtering is applied to all slaves, validation is performed
+ only for backup slaves.
+
+ Validation:
+
+ Enabling validation causes the ARP monitor to examine the incoming
+ ARP requests and replies, and only consider a slave to be up if it
+ is receiving the appropriate ARP traffic.
+
+ For an active slave, the validation checks ARP replies to confirm
+ that they were generated by an arp_ip_target. Since backup slaves
+ do not typically receive these replies, the validation performed
+ for backup slaves is on the broadcast ARP request sent out via the
+ active slave. It is possible that some switch or network
+ configurations may result in situations wherein the backup slaves
+ do not receive the ARP requests; in such a situation, validation
+ of backup slaves must be disabled.
+
+ The validation of ARP requests on backup slaves is mainly helping
+ bonding to decide which slaves are more likely to work in case of
+ the active slave failure, it doesn't really guarantee that the
+ backup slave will work if it's selected as the next active slave.
+
+ Validation is useful in network configurations in which multiple
+ bonding hosts are concurrently issuing ARPs to one or more targets
+ beyond a common switch. Should the link between the switch and
+ target fail (but not the switch itself), the probe traffic
+ generated by the multiple bonding instances will fool the standard
+ ARP monitor into considering the links as still up. Use of
+ validation can resolve this, as the ARP monitor will only consider
+ ARP requests and replies associated with its own instance of
+ bonding.
+
+ Filtering:
+
+ Enabling filtering causes the ARP monitor to only use incoming ARP
+ packets for link availability purposes. Arriving packets that are
+ not ARPs are delivered normally, but do not count when determining
+ if a slave is available.
+
+ Filtering operates by only considering the reception of ARP
+ packets (any ARP packet, regardless of source or destination) when
+ determining if a slave has received traffic for link availability
+ purposes.
+
+ Filtering is useful in network configurations in which significant
+ levels of third party broadcast traffic would fool the standard
+ ARP monitor into considering the links as still up. Use of
+ filtering can resolve this, as only ARP traffic is considered for
+ link availability purposes.
This option was added in bonding version 3.1.0.
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 08/12] bonding: use last_arp_rx in slave_last_rx()
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Now that last_arp_rx really has the last time we've received any (validated or
not) ARP, we can use it in slave_last_rx() instead of slave->dev->last_rx.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bonding.h | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ab2e651..ae20c5a 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -379,14 +379,10 @@ static inline unsigned long slave_oldest_target_arp_rx(struct bonding *bond,
static inline unsigned long slave_last_rx(struct bonding *bond,
struct slave *slave)
{
- if (slave_do_arp_validate(bond, slave)) {
- if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
- return slave_oldest_target_arp_rx(bond, slave);
- else
- return slave->last_arp_rx;
- }
+ if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
+ return slave_oldest_target_arp_rx(bond, slave);
- return slave->dev->last_rx;
+ return slave->last_arp_rx;
}
#ifdef CONFIG_NET_POLL_CONTROLLER
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 09/12] bonding: use last_arp_rx in bond_loadbalance_arp_mon()
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Now that last_arp_rx correctly show the last time we've received an ARP, we
can use it safely instead of slave->dev->last_rx.
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_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3fe81cd..e7aab9a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2372,7 +2372,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
if (slave->link != BOND_LINK_UP) {
if (bond_time_in_interval(bond, trans_start, 1) &&
- bond_time_in_interval(bond, slave->dev->last_rx, 1)) {
+ bond_time_in_interval(bond, slave->last_arp_rx, 1)) {
slave->link = BOND_LINK_UP;
slave_state_changed = 1;
@@ -2401,7 +2401,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
* if we don't know our ip yet
*/
if (!bond_time_in_interval(bond, trans_start, 2) ||
- !bond_time_in_interval(bond, slave->dev->last_rx, 2)) {
+ !bond_time_in_interval(bond, slave->last_arp_rx, 2)) {
slave->link = BOND_LINK_DOWN;
slave_state_changed = 1;
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 10/12] bonding: remove useless updating of slave->dev->last_rx
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
Now that all the logic is handled via last_arp_rx, we don't need to use
last_rx.
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 3 ---
include/linux/netdevice.h | 8 +-------
2 files changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e7aab9a..14e023d 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1115,9 +1115,6 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
slave = bond_slave_get_rcu(skb->dev);
bond = slave->bond;
- if (bond->params.arp_interval)
- slave->dev->last_rx = jiffies;
-
recv_probe = ACCESS_ONCE(bond->recv_probe);
if (recv_probe) {
ret = recv_probe(skb, bond, slave);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 430c51a..891432a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1312,13 +1312,7 @@ struct net_device {
/*
* Cache lines mostly used on receive path (including eth_type_trans())
*/
- unsigned long last_rx; /* Time of last Rx
- * This should not be set in
- * drivers, unless really needed,
- * because network stack (bonding)
- * use it if/when necessary, to
- * avoid dirtying this cache line.
- */
+ unsigned long last_rx; /* Time of last Rx */
/* Interface address info used in eth_type_trans() */
unsigned char *dev_addr; /* hw address, (before bcast
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 11/12] bonding: trivial: rename slave->jiffies to ->last_link_up
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
slave->jiffies is updated every time the slave becomes active, which, for
bonding, means that its link is 'up'.
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_main.c | 20 ++++++++++----------
drivers/net/bonding/bonding.h | 3 ++-
2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 14e023d..f6d56d9 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -798,7 +798,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
return;
if (new_active) {
- new_active->jiffies = jiffies;
+ new_active->last_link_up = jiffies;
if (new_active->link == BOND_LINK_BACK) {
if (USES_PRIMARY(bond->params.mode)) {
@@ -1444,7 +1444,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
}
if (new_slave->link != BOND_LINK_DOWN)
- new_slave->jiffies = jiffies;
+ new_slave->last_link_up = jiffies;
pr_debug("Initial state of slave_dev is BOND_LINK_%s\n",
new_slave->link == BOND_LINK_DOWN ? "DOWN" :
(new_slave->link == BOND_LINK_UP ? "UP" : "BACK"));
@@ -1891,7 +1891,7 @@ static int bond_miimon_inspect(struct bonding *bond)
* recovered before downdelay expired
*/
slave->link = BOND_LINK_UP;
- slave->jiffies = jiffies;
+ slave->last_link_up = jiffies;
pr_info("%s: link status up again after %d ms for interface %s\n",
bond->dev->name,
(bond->params.downdelay - slave->delay) *
@@ -1966,7 +1966,7 @@ static void bond_miimon_commit(struct bonding *bond)
case BOND_LINK_UP:
slave->link = BOND_LINK_UP;
- slave->jiffies = jiffies;
+ slave->last_link_up = jiffies;
if (bond->params.mode == BOND_MODE_8023AD) {
/* prevent it from being the active one */
@@ -2312,7 +2312,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
bond_validate_arp(bond, slave, sip, tip);
else if (bond->curr_active_slave &&
time_after(slave_last_rx(bond, bond->curr_active_slave),
- bond->curr_active_slave->jiffies))
+ bond->curr_active_slave->last_link_up))
bond_validate_arp(bond, slave, tip, sip);
out_unlock:
@@ -2358,9 +2358,9 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
oldcurrent = ACCESS_ONCE(bond->curr_active_slave);
/* see if any of the previous devices are up now (i.e. they have
* xmt and rcv traffic). the curr_active_slave does not come into
- * the picture unless it is null. also, slave->jiffies is not needed
- * here because we send an arp on each slave and give a slave as
- * long as it needs to get the tx/rx within the delta.
+ * the picture unless it is null. also, slave->last_link_up is not
+ * needed here because we send an arp on each slave and give a slave
+ * as long as it needs to get the tx/rx within the delta.
* TODO: what about up/down delay in arp mode? it wasn't here before
* so it can wait
*/
@@ -2486,7 +2486,7 @@ static int bond_ab_arp_inspect(struct bonding *bond)
* active. This avoids bouncing, as the last receive
* times need a full ARP monitor cycle to be updated.
*/
- if (bond_time_in_interval(bond, slave->jiffies, 2))
+ if (bond_time_in_interval(bond, slave->last_link_up, 2))
continue;
/*
@@ -2687,7 +2687,7 @@ static bool bond_ab_arp_probe(struct bonding *bond)
new_slave->link = BOND_LINK_BACK;
bond_set_slave_active_flags(new_slave);
bond_arp_send_all(bond, new_slave);
- new_slave->jiffies = jiffies;
+ new_slave->last_link_up = jiffies;
rcu_assign_pointer(bond->current_arp_slave, new_slave);
rtnl_unlock();
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ae20c5a..36db702 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -188,7 +188,8 @@ struct slave {
struct net_device *dev; /* first - useful for panic debug */
struct bonding *bond; /* our master */
int delay;
- unsigned long jiffies;
+ /* all three in jiffies */
+ unsigned long last_link_up;
unsigned long last_arp_rx;
unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
s8 link; /* one of BOND_LINK_XXXX */
--
1.8.4
^ permalink raw reply related
* [PATCH v3 net-next 12/12] bonding: rename last_arp_rx to last_rx
From: Veaceslav Falico @ 2014-02-17 13:39 UTC (permalink / raw)
To: netdev; +Cc: Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392644361-8510-1-git-send-email-vfalico@redhat.com>
To reflect the new meaning.
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_main.c | 12 ++++++------
drivers/net/bonding/bonding.h | 4 ++--
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index f6d56d9..ac4a1b8 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1397,10 +1397,10 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
bond_update_speed_duplex(new_slave);
- new_slave->last_arp_rx = jiffies -
+ new_slave->last_rx = jiffies -
(msecs_to_jiffies(bond->params.arp_interval) + 1);
for (i = 0; i < BOND_MAX_ARP_TARGETS; i++)
- new_slave->target_last_arp_rx[i] = new_slave->last_arp_rx;
+ new_slave->target_last_arp_rx[i] = new_slave->last_rx;
if (bond->params.miimon && !bond->params.use_carrier) {
link_reporting = bond_check_dev_link(bond, slave_dev, 1);
@@ -2242,7 +2242,7 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
pr_debug("bva: sip %pI4 not found in targets\n", &sip);
return;
}
- slave->last_arp_rx = jiffies;
+ slave->last_rx = jiffies;
slave->target_last_arp_rx[i] = jiffies;
}
@@ -2257,7 +2257,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
if (!slave_do_arp_validate(bond, slave)) {
if ((slave_do_arp_validate_only(bond, slave) && is_arp) ||
!slave_do_arp_validate_only(bond, slave))
- slave->last_arp_rx = jiffies;
+ slave->last_rx = jiffies;
return RX_HANDLER_ANOTHER;
} else if (!is_arp) {
return RX_HANDLER_ANOTHER;
@@ -2369,7 +2369,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
if (slave->link != BOND_LINK_UP) {
if (bond_time_in_interval(bond, trans_start, 1) &&
- bond_time_in_interval(bond, slave->last_arp_rx, 1)) {
+ bond_time_in_interval(bond, slave->last_rx, 1)) {
slave->link = BOND_LINK_UP;
slave_state_changed = 1;
@@ -2398,7 +2398,7 @@ static void bond_loadbalance_arp_mon(struct work_struct *work)
* if we don't know our ip yet
*/
if (!bond_time_in_interval(bond, trans_start, 2) ||
- !bond_time_in_interval(bond, slave->last_arp_rx, 2)) {
+ !bond_time_in_interval(bond, slave->last_rx, 2)) {
slave->link = BOND_LINK_DOWN;
slave_state_changed = 1;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 36db702..4303628 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -190,7 +190,7 @@ struct slave {
int delay;
/* all three in jiffies */
unsigned long last_link_up;
- unsigned long last_arp_rx;
+ unsigned long last_rx;
unsigned long target_last_arp_rx[BOND_MAX_ARP_TARGETS];
s8 link; /* one of BOND_LINK_XXXX */
s8 new_link;
@@ -383,7 +383,7 @@ static inline unsigned long slave_last_rx(struct bonding *bond,
if (bond->params.arp_all_targets == BOND_ARP_TARGETS_ALL)
return slave_oldest_target_arp_rx(bond, slave);
- return slave->last_arp_rx;
+ return slave->last_rx;
}
#ifdef CONFIG_NET_POLL_CONTROLLER
--
1.8.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox