* [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: rama nichanamatlu @ 2013-11-21 0:53 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 4588 bytes --]
During the creation of VLAN's atop bonding the underlying interfaces are
made part of VLAN's, and at the same bonding driver gets aware that
VLAN's exists above it and hence would consult IP routing for every ARP
to be sent to determine the route which tells bonding driver the
correct VLAN tag to attach to the outgoing ARP packet. But, during the
VLAN creation when vlan driver puts the underlying interface into
default vlan and then actual vlan, in-between this if bonding driver
consults the IP for a route, IP fails to provide a correct route and
upon which bonding driver drops the ARP packet. ARP monitor when it
comes around next time, sees no ARP response and fails-over to the next
available slave. Consulting for a IP route, ip_route_output(),happens in
bond_arp_send_all().
To prevent this false fail-over, when bonding driver fails to send an
ARP out it marks in its private structure, bonding{}, not to expect an
ARP response, when ARP monitor comes around next time ARP sending will
be tried again.
Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan
intf. All virtual interfaces created at boot time.
Orabug: 17172660
Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
---
drivers/net/bonding/bond_main.c | 13 ++++++++-----
drivers/net/bonding/bonding.h | 1 +
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index dde6b4a..d475161 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond,
__be32 ip)
* switches in VLAN mode (especially if ports are configured as
* "native" to a VLAN) might not pass non-tagged frames.
*/
-static void bond_arp_send(struct net_device *slave_dev, int arp_op,
__be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
+static void bond_arp_send(struct bonding *bond, struct net_device
*slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short
vlan_id)
{
struct sk_buff *skb;
@@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device
*slave_dev, int arp_op, __be32 dest_
}
}
arp_xmit(skb);
+ bond->arp_sent=true;
}
@@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
pr_debug("basa: target %x\n", targets[i]);
if (!bond->vlgrp) {
pr_debug("basa: empty vlan: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (rt->dst.dev == bond->dev) {
ip_rt_put(rt);
pr_debug("basa: rtdev == bond->dev: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (vlan_id) {
ip_rt_put(rt);
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
vlan->vlan_ip, vlan_id);
continue;
}
@@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
should_notify_peers = bond_should_notify_peers(bond);
- if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
+ if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) {
read_unlock(&bond->lock);
rtnl_lock();
read_lock(&bond->lock);
@@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
read_lock(&bond->lock);
}
+ bond->arp_sent=false;
bond_ab_arp_probe(bond);
re_arm:
@@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev)
bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM);
bond_dev->features |= bond_dev->hw_features;
+ bond->arp_sent=false;
}
static void bond_work_cancel_all(struct bonding *bond)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e9a3c56..3878bbd 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -253,6 +253,7 @@ struct bonding {
/* debugging suport via debugfs */
struct dentry *debug_dir;
#endif /* CONFIG_DEBUG_FS */
+ bool arp_sent;
};
#define bond_slave_get_rcu(dev) \
--
1.8.2.1
[-- Attachment #2: Attached Message Part --]
[-- Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related
* Re: [PATCH] net: rework recvmsg handler msg_name and msg_namelen logic
From: Joe Perches @ 2013-11-21 1:02 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, davem, eric.dumazet
In-Reply-To: <20131121003828.GA32587@order.stressinduktion.org>
On Thu, 2013-11-21 at 01:38 +0100, Hannes Frederic Sowa wrote:
> This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> to return msg_name to the user.
[]
> This change does not alter the user visible error logic as we ignore
> msg_namelen as long as msg_name is NULL.
>
> Also remove two unnecessary curly brackets in ___sys_recvmsg and change
> comments to netdev style.
Perhaps take the opportunity to remove the unnecessary casts of
struct msghdr.msg_name as it's a void *.
And there's one other oddity about setting a known NULL to NULL.
Maybe fix in a follow-on patch?
> diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
[]
> @@ -135,9 +134,10 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
[]
> + if (msg->msg_name) {
> + struct sockaddr_mISDN *maddr =
> + (struct sockaddr_mISDN *)msg->msg_name;
struct sockaddr_mISDN *maddr = msg->msg_name;
> diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
[]
> @@ -1764,14 +1763,14 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
[]
> + if (!err && msg->msg_name) {
> + struct sockaddr_at *sat =
> + (struct sockaddr_at *)msg->msg_name;
> diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
[]
> @@ -1636,11 +1636,12 @@ static int ax25_recvmsg(struct kiocb *iocb, struct socket *sock,
[]
> + struct sockaddr_ax25 *sax =
> + (struct sockaddr_ax25 *)msg->msg_name;
> diff --git a/net/core/iovec.c b/net/core/iovec.c
[]
> @@ -48,7 +48,7 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
> if (err < 0)
> return err;
> }
> - m->msg_name = address;
> + m->msg_name = (m->msg_name) ? address : NULL;
It's pretty odd I think to set a known NULL to NULL;
> diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
[]
> @@ -1252,8 +1251,11 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock,
[]
> + srose = (struct sockaddr_rose *)msg->msg_name;
^ permalink raw reply
* Re: [PATCH] net: rework recvmsg handler msg_name and msg_namelen logic
From: Hannes Frederic Sowa @ 2013-11-21 1:07 UTC (permalink / raw)
To: Joe Perches; +Cc: netdev, davem, eric.dumazet
In-Reply-To: <1384995760.2164.33.camel@joe-AO722>
On Wed, Nov 20, 2013 at 05:02:40PM -0800, Joe Perches wrote:
> On Thu, 2013-11-21 at 01:38 +0100, Hannes Frederic Sowa wrote:
> > This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> > set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> > to return msg_name to the user.
> []
> > This change does not alter the user visible error logic as we ignore
> > msg_namelen as long as msg_name is NULL.
> >
> > Also remove two unnecessary curly brackets in ___sys_recvmsg and change
> > comments to netdev style.
>
> Perhaps take the opportunity to remove the unnecessary casts of
> struct msghdr.msg_name as it's a void *.
>
> And there's one other oddity about setting a known NULL to NULL.
>
> Maybe fix in a follow-on patch?
No, that is easily done in this one. I'll send a v2 shortly.
Thanks!
^ permalink raw reply
* Re: Get rxhash fixes and RFS support in tun
From: Tom Herbert @ 2013-11-21 1:09 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, Eric Dumazet, Jerry Chu
In-Reply-To: <20131120.190252.940036939480739452.davem@davemloft.net>
On Wed, Nov 20, 2013 at 4:02 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Wed, 20 Nov 2013 12:25:48 -0800 (PST)
>
>> This patch series fixes some subtle bugs in tun use of skb->rxhash, all
>> rxhash hash not be cleared appropraitely, and adds support for tun flows
>> to work with RFS.
>>
>> Testing, in particular with tun, hasn't been completed yet.
>
> I think this needs to be reworked slightly.
>
> We really only have two boolean states:
>
> 1) Is the rxhash value in this SKB valid?
>
> 2) Is it a full L4 tuple hash?
>
> You are adding a "this is a SW computed hash" boolean state but I do
> not think you should distinguish sw vs. hw especially. If the
> hardware computed the rxhash on a tunneled packet in the
> pre-decapsulated state, we very much want to recompute it, in
> software, upon tunnel decapsulation in ip_tunnel_core.c
>
In either case it would be recomputed in SW if L4 hash was not set
(i.e. no flow_dissector done finding L4). If L4 hash is set, that
should refer to the hash of the inner 4-tuple, so I don't think you'd
need to recompute it. I suppose there could be a case like encap in
UDP where there are potentially two 4-tuples to deal with, but then
the rxhash could just be cleared in decap to force recomputing hash
over the inner packet-- even better I would hope that the trick of
using outer source port to hold the hash of the inner packet (like in
nvgre) is used so that the hash on the outer header is good for L4.
The primary reason for sw_rhash is to know whether it is a comparable
value to match against that of a flow whose hash was computed in SW
(tun case).
> This is actually implemented in this patch set, by testing two states.
> Both the "l4_rxhash" and "sw_rxhash".
>
> Why don't we just do everything in a straightforward manner, where
> nothing directly sets rxhash values. Only helper routines do.
>
> 1) skb_set_rxhash(struct sk_buff *skb, __u32 rxhash, bool l4_rxhash)
>
Yes, we should be using helper functions for this anyway. I'll make
that change.
> Update all drivers to call this.
>
> 2) Add "rxhash_valid" boolean to sk_buff, set it in skb_set_rxhash,
> test it in skb_get_rxhash(), propagate it in SKB copies.
>
I think the sw_rxhash holds more useful information. In
skb_get_rxhash() either we have an L4 hash or we try to get one by
doing the SW computation, this means we don't ever return a HW hash
which is not L4 so basically that case is treated as an invalid hash.
So after a call to skb_get_rxhash, at least one of l4_rxhash or
sw_rxhash is and we won't redo flow dissection on subsequent calls
unless the rxhash is cleared.
Thanks,
Tom
> 3) Your skb_clear_rxhash() now just clears rxhash_valid.
>
> Now, if the issue is that HW computed hashes sometimes do the tunnel
> demux and look into the inner L4 headers to compute the hash, you'll
> need a boolean to indicate _that_ rather than unconditionally treating
> hardware that way. Because not all of them will do this, and for
> those that do not you do want to compute the hash in SW after tunnel
> decapsulation.
>
> Thoughts?
^ permalink raw reply
* Re: [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: Ding Tianhong @ 2013-11-21 1:10 UTC (permalink / raw)
To: rama nichanamatlu, netdev
In-Reply-To: <528D5980.3040309@oracle.com>
On 2013/11/21 8:53, rama nichanamatlu wrote:
> During the creation of VLAN's atop bonding the underlying interfaces are made part of VLAN's, and at the same bonding driver gets aware that VLAN's exists above it and hence would consult IP routing for every ARP to be sent to determine the route which tells bonding driver the correct VLAN tag to attach to the outgoing ARP packet. But, during the VLAN creation when vlan driver puts the underlying interface into default vlan and then actual vlan, in-between this if bonding driver consults the IP for a route, IP fails to provide a correct route and upon which bonding driver drops the ARP packet. ARP monitor when it
> comes around next time, sees no ARP response and fails-over to the next available slave. Consulting for a IP route, ip_route_output(),happens in bond_arp_send_all().
>
> To prevent this false fail-over, when bonding driver fails to send an ARP out it marks in its private structure, bonding{}, not to expect an ARP response, when ARP monitor comes around next time ARP sending will be tried again.
>
> Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan intf. All virtual interfaces created at boot time.
>
please reorganize the changelog, it is too log for a line, please make it in 80 chats.
> Orabug: 17172660
> Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
> Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
> ---
> drivers/net/bonding/bond_main.c | 13 ++++++++-----
> drivers/net/bonding/bonding.h | 1 +
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index dde6b4a..d475161 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond, __be32 ip)
> * switches in VLAN mode (especially if ports are configured as
> * "native" to a VLAN) might not pass non-tagged frames.
> */
> -static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
> +static void bond_arp_send(struct bonding *bond, struct net_device *slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
no need to add bond, the bond could be get from slave, see slave->bond
Ding
> {
> struct sk_buff *skb;
> @@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device *slave_dev, int arp_op, __be32 dest_
> }
> }
> arp_xmit(skb);
> + bond->arp_sent=true;
> }
> @@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> pr_debug("basa: target %x\n", targets[i]);
> if (!bond->vlgrp) {
> pr_debug("basa: empty vlan: arp_send\n");
> - bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> + bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> bond->master_ip, 0);
> continue;
> }
> @@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> if (rt->dst.dev == bond->dev) {
> ip_rt_put(rt);
> pr_debug("basa: rtdev == bond->dev: arp_send\n");
> - bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> + bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> bond->master_ip, 0);
> continue;
> }
> @@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> if (vlan_id) {
> ip_rt_put(rt);
> - bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> + bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> vlan->vlan_ip, vlan_id);
> continue;
> }
> @@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct *work)
> should_notify_peers = bond_should_notify_peers(bond);
> - if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
> + if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) {
> read_unlock(&bond->lock);
> rtnl_lock();
> read_lock(&bond->lock);
> @@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct *work)
> read_lock(&bond->lock);
> }
> + bond->arp_sent=false;
> bond_ab_arp_probe(bond);
> re_arm:
> @@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev)
> bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM);
> bond_dev->features |= bond_dev->hw_features;
> + bond->arp_sent=false;
> }
> static void bond_work_cancel_all(struct bonding *bond)
> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
> index e9a3c56..3878bbd 100644
> --- a/drivers/net/bonding/bonding.h
> +++ b/drivers/net/bonding/bonding.h
> @@ -253,6 +253,7 @@ struct bonding {
> /* debugging suport via debugfs */
> struct dentry *debug_dir;
> #endif /* CONFIG_DEBUG_FS */
> + bool arp_sent;
> };
> #define bond_slave_get_rcu(dev) \
^ permalink raw reply
* Re: Get rxhash fixes and RFS support in tun
From: Stephen Hemminger @ 2013-11-21 1:12 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Linux Netdev List, Eric Dumazet, Jerry Chu
In-Reply-To: <CA+mtBx91hSmYKrbUsPrtnXRjYR8QDGZ+GAAnMixauUqw9LFkiQ@mail.gmail.com>
On Wed, 20 Nov 2013 17:09:01 -0800
Tom Herbert <therbert@google.com> wrote:
> On Wed, Nov 20, 2013 at 4:02 PM, David Miller <davem@davemloft.net> wrote:
> > From: Tom Herbert <therbert@google.com>
> > Date: Wed, 20 Nov 2013 12:25:48 -0800 (PST)
> >
> >> This patch series fixes some subtle bugs in tun use of skb->rxhash, all
> >> rxhash hash not be cleared appropraitely, and adds support for tun flows
> >> to work with RFS.
> >>
> >> Testing, in particular with tun, hasn't been completed yet.
> >
> > I think this needs to be reworked slightly.
> >
> > We really only have two boolean states:
> >
> > 1) Is the rxhash value in this SKB valid?
> >
> > 2) Is it a full L4 tuple hash?
> >
> > You are adding a "this is a SW computed hash" boolean state but I do
> > not think you should distinguish sw vs. hw especially. If the
> > hardware computed the rxhash on a tunneled packet in the
> > pre-decapsulated state, we very much want to recompute it, in
> > software, upon tunnel decapsulation in ip_tunnel_core.c
> >
> In either case it would be recomputed in SW if L4 hash was not set
> (i.e. no flow_dissector done finding L4). If L4 hash is set, that
> should refer to the hash of the inner 4-tuple, so I don't think you'd
> need to recompute it. I suppose there could be a case like encap in
> UDP where there are potentially two 4-tuples to deal with, but then
> the rxhash could just be cleared in decap to force recomputing hash
> over the inner packet-- even better I would hope that the trick of
> using outer source port to hold the hash of the inner packet (like in
> nvgre) is used so that the hash on the outer header is good for L4.
>
> The primary reason for sw_rhash is to know whether it is a comparable
> value to match against that of a flow whose hash was computed in SW
> (tun case).
You can't guarantee that two hardware hashes from different devices
are the same.
^ permalink raw reply
* Re: [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: Jay Vosburgh @ 2013-11-21 1:18 UTC (permalink / raw)
To: rama nichanamatlu; +Cc: netdev
In-Reply-To: <528D5980.3040309@oracle.com>
rama nichanamatlu <rama.nichanamatlu@oracle.com> wrote:
>During the creation of VLAN's atop bonding the underlying interfaces are
>made part of VLAN's, and at the same bonding driver gets aware that VLAN's
>exists above it and hence would consult IP routing for every ARP to be
>sent to determine the route which tells bonding driver the correct VLAN
>tag to attach to the outgoing ARP packet. But, during the VLAN creation
>when vlan driver puts the underlying interface into default vlan and then
>actual vlan, in-between this if bonding driver consults the IP for a
>route, IP fails to provide a correct route and upon which bonding driver
>drops the ARP packet. ARP monitor when it
>comes around next time, sees no ARP response and fails-over to the next
>available slave. Consulting for a IP route, ip_route_output(),happens in
>bond_arp_send_all().
>
>To prevent this false fail-over, when bonding driver fails to send an ARP
>out it marks in its private structure, bonding{}, not to expect an ARP
>response, when ARP monitor comes around next time ARP sending will be
>tried again.
>
>Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan
>intf. All virtual interfaces created at boot time.
First, this patch appears to be for an older kernel, as the
current mainline code is substantially different (e.g., master_ip is no
longer used).
Second, won't this methodology mask legitimate failures, such as
when a single arp_ip_target specifies a destination that is not ever
reachable? I.e., would specifying a permanently unreachable IP address
as the arp_ip_target cause all slaves to always stay up (because no ARPs
will ever be sent), even if no ARP replies are ever received?
-J
>Orabug: 17172660
>Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
>Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
>---
> drivers/net/bonding/bond_main.c | 13 ++++++++-----
> drivers/net/bonding/bonding.h | 1 +
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c
>b/drivers/net/bonding/bond_main.c
>index dde6b4a..d475161 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond,
>__be32 ip)
> * switches in VLAN mode (especially if ports are configured as
> * "native" to a VLAN) might not pass non-tagged frames.
> */
>-static void bond_arp_send(struct net_device *slave_dev, int arp_op,
>__be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
>+static void bond_arp_send(struct bonding *bond, struct net_device
>*slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short
>vlan_id)
> {
> struct sk_buff *skb;
> @@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device
>*slave_dev, int arp_op, __be32 dest_
> }
> }
> arp_xmit(skb);
>+ bond->arp_sent=true;
> }
> @@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding
>*bond, struct slave *slave)
> pr_debug("basa: target %x\n", targets[i]);
> if (!bond->vlgrp) {
> pr_debug("basa: empty vlan: arp_send\n");
>- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
>+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> bond->master_ip, 0);
> continue;
> }
>@@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding *bond,
>struct slave *slave)
> if (rt->dst.dev == bond->dev) {
> ip_rt_put(rt);
> pr_debug("basa: rtdev == bond->dev: arp_send\n");
>- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
>+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> bond->master_ip, 0);
> continue;
> }
>@@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding *bond,
>struct slave *slave)
> if (vlan_id) {
> ip_rt_put(rt);
>- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
>+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
> vlan->vlan_ip, vlan_id);
> continue;
> }
>@@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct
>*work)
> should_notify_peers = bond_should_notify_peers(bond);
> - if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
>+ if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) {
> read_unlock(&bond->lock);
> rtnl_lock();
> read_lock(&bond->lock);
>@@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct
>*work)
> read_lock(&bond->lock);
> }
> + bond->arp_sent=false;
> bond_ab_arp_probe(bond);
> re_arm:
>@@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev)
> bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM);
> bond_dev->features |= bond_dev->hw_features;
>+ bond->arp_sent=false;
> }
> static void bond_work_cancel_all(struct bonding *bond)
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index e9a3c56..3878bbd 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -253,6 +253,7 @@ struct bonding {
> /* debugging suport via debugfs */
> struct dentry *debug_dir;
> #endif /* CONFIG_DEBUG_FS */
>+ bool arp_sent;
> };
> #define bond_slave_get_rcu(dev) \
>--
>1.8.2.1
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] net: rework recvmsg handler msg_name and msg_namelen logic
From: Eric Dumazet @ 2013-11-21 1:18 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Joe Perches, netdev, davem
In-Reply-To: <20131121010729.GB32587@order.stressinduktion.org>
On Thu, 2013-11-21 at 02:07 +0100, Hannes Frederic Sowa wrote:
> On Wed, Nov 20, 2013 at 05:02:40PM -0800, Joe Perches wrote:
> > On Thu, 2013-11-21 at 01:38 +0100, Hannes Frederic Sowa wrote:
> > > This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> > > set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> > > to return msg_name to the user.
> > []
> > > This change does not alter the user visible error logic as we ignore
> > > msg_namelen as long as msg_name is NULL.
> > >
> > > Also remove two unnecessary curly brackets in ___sys_recvmsg and change
> > > comments to netdev style.
> >
> > Perhaps take the opportunity to remove the unnecessary casts of
> > struct msghdr.msg_name as it's a void *.
> >
> > And there's one other oddity about setting a known NULL to NULL.
> >
> > Maybe fix in a follow-on patch?
>
> No, that is easily done in this one. I'll send a v2 shortly.
>
> Thanks!
>
In move_addr_to_user() there is this strange test :
if (len > klen)
len = klen;
if (len < 0 || len > sizeof(struct sockaddr_storage))
return -EINVAL;
I would suggest to instead use :
if (len < 0)
return -EINVAL;
BUG_ON(len > sizeof(struct sockaddr_storage));
To catch if some protocols are using more than 128 bytes.
We can then remove the BUG_ON() later.
^ permalink raw reply
* Re: [PATCH] net: rework recvmsg handler msg_name and msg_namelen logic
From: Hannes Frederic Sowa @ 2013-11-21 1:23 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Joe Perches, netdev, davem
In-Reply-To: <1384996730.10637.28.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, Nov 20, 2013 at 05:18:50PM -0800, Eric Dumazet wrote:
> On Thu, 2013-11-21 at 02:07 +0100, Hannes Frederic Sowa wrote:
> > On Wed, Nov 20, 2013 at 05:02:40PM -0800, Joe Perches wrote:
> > > On Thu, 2013-11-21 at 01:38 +0100, Hannes Frederic Sowa wrote:
> > > > This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> > > > set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> > > > to return msg_name to the user.
> > > []
> > > > This change does not alter the user visible error logic as we ignore
> > > > msg_namelen as long as msg_name is NULL.
> > > >
> > > > Also remove two unnecessary curly brackets in ___sys_recvmsg and change
> > > > comments to netdev style.
> > >
> > > Perhaps take the opportunity to remove the unnecessary casts of
> > > struct msghdr.msg_name as it's a void *.
> > >
> > > And there's one other oddity about setting a known NULL to NULL.
> > >
> > > Maybe fix in a follow-on patch?
> >
> > No, that is easily done in this one. I'll send a v2 shortly.
> >
> > Thanks!
> >
>
> In move_addr_to_user() there is this strange test :
>
> if (len > klen)
> len = klen;
> if (len < 0 || len > sizeof(struct sockaddr_storage))
> return -EINVAL;
>
> I would suggest to instead use :
>
> if (len < 0)
> return -EINVAL;
> BUG_ON(len > sizeof(struct sockaddr_storage));
>
> To catch if some protocols are using more than 128 bytes.
>
> We can then remove the BUG_ON() later.
I actually started adding a handful BUILD_BUG_ONs on my branch as I don't know
if we will catch all corner-cases at runtime by the users and planed that for
net-next. But it is better to get such a check in as early as possible, so
I'll include it in v2.
Thanks!
^ permalink raw reply
* Re: Fw: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
From: Eric Dumazet @ 2013-11-21 1:35 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: julien.delacroix, netdev
In-Reply-To: <1384980072.10637.25.camel@edumazet-glaptop2.roam.corp.google.com>
On Wed, 2013-11-20 at 12:41 -0800, Eric Dumazet wrote:
> On Wed, 2013-11-20 at 12:22 -0800, Stephen Hemminger wrote:
> >
> > Begin forwarded message:
> >
> > Date: Wed, 20 Nov 2013 07:03:59 -0800
> > From: "bugzilla-daemon@bugzilla.kernel.org" <bugzilla-daemon@bugzilla.kernel.org>
> > To: "stephen@networkplumber.org" <stephen@networkplumber.org>
> > Subject: [Bug 65271] New: Acceptable RST packets (with seq number in window) are ignored in LAST_ACK state (and previously sent data pending acknowledgement)
> >
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=65271
> >
> > Bug ID: 65271
> > Summary: Acceptable RST packets (with seq number in window) are
> > ignored in LAST_ACK state (and previously sent data
> > pending acknowledgement)
> > Product: Networking
> > Version: 2.5
> > Kernel Version: 2.6.32.59-0.7-default
> > Hardware: All
> > OS: Linux
> > Tree: Mainline
> > Status: NEW
> > Severity: normal
> > Priority: P1
> > Component: IPV4
> > Assignee: shemminger@linux-foundation.org
> > Reporter: julien.delacroix@gmail.com
> > Regression: No
> >
> > Created attachment 115271
> > --> https://bugzilla.kernel.org/attachment.cgi?id=115271&action=edit
> > TCP dump showing described scenario
> >
> > PCAP file of traffic capture is attached.
> > Note that capture was taken directly on server host, so RST packets definitely
> > reached the server host.
> >
> >
> > Here is the flow:
> >
> > 1: Client ---- SYN ----> Server
> > 2: Client <- SYN, ACK -- Server
> > 3: Client ---- ACK ----> Server
> > 4: Client ---- FIN ----> Server ( server in CLOSE_WAIT state)
> > 5: Client <- PSH, ACK -- Server
> > 6: Client <- FIN, ACK -- Server ( server in LAST_ACK state)
> >
> > 7: Client ---- RST ----> Server
> >
> > Then repeat N times:
> > 8: Client <- PSH, ACK -- Server
> > 9: Client ---RST, ACK -> Server
> >
> >
> > Notes:
> > 5: packet contains 3 bytes of data
> > 7: no ACK flag, and sequence number is correct, but RST packet is ignored
> > 8: retransmission of step 5
> > 9: the sequence number is correct as well as the acknowledgment number
> > (acknowledging data in 5, but not yet FIN in 6), but this RST is again ignored.
> >
> > From this point client keeps retransmitting 5, while server keeps answering
> > with acceptable RST packets.
> >
> > You can observe this scenario (with 5 retransmissions and 5 RST) in the
> > attached PCAP file.
> >
>
> Thanks for the report, you'll take a look.
>
I could not reproduce the issue here (using latest David Miller net
tree) :
17:28:57.643335 IP 192.0.2.1.47104 > 192.168.0.1.8080: Flags [S], seq 0, win 32792, options [mss 1460,sackOK,nop,nop,nop,wscale 7], length 0
17:28:57.643382 IP 192.168.0.1.8080 > 192.0.2.1.47104: Flags [S.], seq 3351683989, ack 1, win 29200, options [mss 1460,nop,nop,sackOK,nop,wscale 6], length 0
17:28:57.743309 IP 192.0.2.1.47104 > 192.168.0.1.8080: Flags [.], ack 1, win 257, length 0
17:28:57.753309 IP 192.0.2.1.47104 > 192.168.0.1.8080: Flags [F.], seq 1, ack 1, win 46, length 0
17:28:57.754053 IP 192.168.0.1.8080 > 192.0.2.1.47104: Flags [.], ack 2, win 457, length 0
17:28:57.793318 IP 192.168.0.1.8080 > 192.0.2.1.47104: Flags [P.], seq 1:4, ack 2, win 457, length 3
17:28:57.794279 IP 192.168.0.1.8080 > 192.0.2.1.47104: Flags [F.], seq 4, ack 2, win 457, length 0
17:28:57.843310 IP 192.0.2.1.47104 > 192.168.0.1.8080: Flags [R], seq 4168304244, win 0, length 0
17:28:58.295101 IP 192.168.0.1.8080 > 192.0.2.1.47104: Flags [P.], seq 1:4, ack 2, win 457, length 3
17:28:58.710341 IP 192.0.2.1.47104 > 192.168.0.1.8080: Flags [R.], seq 2, ack 4, win 0, length 0
Note that first RST is ignored as its sequence number is out of window.
But the 2nd one is correctly taken by TCP stack
I suspect a netfilter bug (conntrack), 'accepting' the buggy RST packet,
and blocking the other RST.
packetdrill scenario I cooked was :
`../common/defaults.sh`
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
0.000 bind(3, ..., ...) = 0
0.000 listen(3, 1) = 0
0.100 < S 0:0(0) win 32792 <mss 1460,sackOK,nop,nop,nop,wscale 7>
0.100 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 6>
0.200 < . 1:1(0) ack 1 win 257
0.200 accept(3, ..., ...) = 4
// Receive FIN segment.
0.210 < F. 1:1(0) ack 1 win 46
// Send one ACK.
+0 > . 1:1(0) ack 2
// Application writes 3 bytes.
0.250 write(4, ..., 3) = 3
+0 > P. 1:4(3) ack 2
0.251 shutdown(4, 2) = 0
+0 > F. 4:4(0) ack 2
// RST : it is ignored because its sequence is out of window
0.300 < R 4168304244:4168304244(0) win 0
// retransmit
0.751 > P. 1:4(3) ack 2
// This RST is OK and accepted
1.167 < R. 2:2(0) ack 4 win 0
// This retransmit should not happen
4.166 > P. 1:4(3) ack 2
^ permalink raw reply
* [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: rama nichanamatlu @ 2013-11-21 1:36 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 4513 bytes --]
During the creation of VLAN's atop bonding the underlying interfaces are
made part of VLAN's, and at the same bonding driver gets aware that
VLAN's exists above it and hence would consult IP routing for every ARP
to be sent to determine the route which tells bonding driver the
correct VLAN tag to attach to the outgoing ARP packet. But, during the
VLAN creation when vlan driver puts the underlying interface into
default vlan and then actual vlan, in-between this if bonding driver
consults the IP for a route, IP fails to provide a correct route and
upon which bonding driver drops the ARP packet. ARP monitor when it
comes around next time, sees no ARP response and fails-over to the next
available slave. Consulting for a IP route, ip_route_output(),happens in
bond_arp_send_all().
To prevent this false fail-over, when bonding driver fails to send an
ARP out it marks in its private structure, bonding{}, not to expect an
ARP response, when ARP monitor comes around next time ARP sending will
be tried again.
Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan
intf. All virtual interfaces created at boot time.
Orabug: 17172660
Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
---
drivers/net/bonding/bond_main.c | 13 ++++++++-----
drivers/net/bonding/bonding.h | 1 +
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index dde6b4a..d475161 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond,
__be32 ip)
* switches in VLAN mode (especially if ports are configured as
* "native" to a VLAN) might not pass non-tagged frames.
*/
-static void bond_arp_send(struct net_device *slave_dev, int arp_op,
__be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
+static void bond_arp_send(struct bonding *bond, struct net_device
*slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short
vlan_id)
{
struct sk_buff *skb;
@@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device
*slave_dev, int arp_op, __be32 dest_
}
}
arp_xmit(skb);
+ bond->arp_sent=true;
}
@@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
pr_debug("basa: target %x\n", targets[i]);
if (!bond->vlgrp) {
pr_debug("basa: empty vlan: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (rt->dst.dev == bond->dev) {
ip_rt_put(rt);
pr_debug("basa: rtdev == bond->dev: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (vlan_id) {
ip_rt_put(rt);
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
vlan->vlan_ip, vlan_id);
continue;
}
@@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
should_notify_peers = bond_should_notify_peers(bond);
- if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
+ if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) {
read_unlock(&bond->lock);
rtnl_lock();
read_lock(&bond->lock);
@@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
read_lock(&bond->lock);
}
+ bond->arp_sent=false;
bond_ab_arp_probe(bond);
re_arm:
@@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev)
bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM);
bond_dev->features |= bond_dev->hw_features;
+ bond->arp_sent=false;
}
static void bond_work_cancel_all(struct bonding *bond)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e9a3c56..3878bbd 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -253,6 +253,7 @@ struct bonding {
/* debugging suport via debugfs */
struct dentry *debug_dir;
#endif /* CONFIG_DEBUG_FS */
+ bool arp_sent;
};
#define bond_slave_get_rcu(dev) \
--
1.8.2.1
[-- Attachment #2: Attached Message Part --]
[-- Type: text/plain, Size: 0 bytes --]
^ permalink raw reply related
* [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: rama nichanamatlu @ 2013-11-21 1:40 UTC (permalink / raw)
To: netdev; +Cc: NICHANAMATLU
In-Reply-To: <528D5DF7.6060103@oracle.com>
During the creation of VLAN's atop bonding the underlying interfaces are
made part of VLAN's, and at the same bonding driver gets aware that
VLAN's exists above it and hence would consult IP routing for every ARP
to be sent to determine the route which tells bonding driver the
correct VLAN tag for the outgoing ARP packet. But, during the
VLAN creation when vlan driver puts the underlying interface into
default vlan and then actual vlan, in-between this if bonding driver
consults the IP for a route, IP fails to provide a correct route and
upon which bonding driver drops the ARP packet. ARP monitor when it
comes around next time, sees no ARP response and fails-over to the next
available slave. Consulting for a IP route, ip_route_output(),happens in
bond_arp_send_all().
To prevent this false fail-over, when bonding driver fails to send an
ARP out it marks in its private structure, bonding{}, not to expect an
ARP response, when ARP monitor comes around next time ARP sending will
be tried again.
Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan
intf. All virtual interfaces created at boot time.
Orabug: 17172660
Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
---
drivers/net/bonding/bond_main.c | 13 ++++++++-----
drivers/net/bonding/bonding.h | 1 +
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index dde6b4a..d475161 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2661,7 +2661,7 @@ static int bond_has_this_ip(struct bonding *bond,
__be32 ip)
* switches in VLAN mode (especially if ports are configured as
* "native" to a VLAN) might not pass non-tagged frames.
*/
-static void bond_arp_send(struct net_device *slave_dev, int arp_op,
__be32 dest_ip, __be32 src_ip, unsigned short vlan_id)
+static void bond_arp_send(struct bonding *bond, struct net_device
*slave_dev, int arp_op, __be32 dest_ip, __be32 src_ip, unsigned short
vlan_id)
{
struct sk_buff *skb;
@@ -2683,6 +2683,7 @@ static void bond_arp_send(struct net_device
*slave_dev, int arp_op, __be32 dest_
}
}
arp_xmit(skb);
+ bond->arp_sent=true;
}
@@ -2700,7 +2701,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
pr_debug("basa: target %x\n", targets[i]);
if (!bond->vlgrp) {
pr_debug("basa: empty vlan: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2726,7 +2727,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (rt->dst.dev == bond->dev) {
ip_rt_put(rt);
pr_debug("basa: rtdev == bond->dev: arp_send\n");
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
bond->master_ip, 0);
continue;
}
@@ -2744,7 +2745,7 @@ static void bond_arp_send_all(struct bonding
*bond, struct slave *slave)
if (vlan_id) {
ip_rt_put(rt);
- bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
+ bond_arp_send(bond, slave->dev, ARPOP_REQUEST, targets[i],
vlan->vlan_ip, vlan_id);
continue;
}
@@ -3206,7 +3207,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
should_notify_peers = bond_should_notify_peers(bond);
- if (bond_ab_arp_inspect(bond, delta_in_ticks)) {
+ if (bond->arp_sent && bond_ab_arp_inspect(bond, delta_in_ticks)) {
read_unlock(&bond->lock);
rtnl_lock();
read_lock(&bond->lock);
@@ -3218,6 +3219,7 @@ void bond_activebackup_arp_mon(struct work_struct
*work)
read_lock(&bond->lock);
}
+ bond->arp_sent=false;
bond_ab_arp_probe(bond);
re_arm:
@@ -4425,6 +4427,7 @@ static void bond_setup(struct net_device *bond_dev)
bond_dev->hw_features &= ~(NETIF_F_ALL_CSUM & ~NETIF_F_NO_CSUM);
bond_dev->features |= bond_dev->hw_features;
+ bond->arp_sent=false;
}
static void bond_work_cancel_all(struct bonding *bond)
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e9a3c56..3878bbd 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -253,6 +253,7 @@ struct bonding {
/* debugging suport via debugfs */
struct dentry *debug_dir;
#endif /* CONFIG_DEBUG_FS */
+ bool arp_sent;
};
#define bond_slave_get_rcu(dev) \
--
1.8.2.1
^ permalink raw reply related
* [PATCH 0/4] Add ceph root filesystem functionality and documentation.
From: mark.doffman @ 2013-11-21 2:13 UTC (permalink / raw)
To: ceph-devel
Cc: Mark Doffman, sage, netdev, linux-kernel, linux-nfs, rob.taylor
From: Mark Doffman <mark.doffman@codethink.co.uk>
Hi All,
The following patch series adds the ability to use a ceph distributed
file system as the root device. The functionality is similar to
NFS root but for the ceph filesystem.
The patch series adds a new root device option '/dev/ceph'.
Configuration is passed either via a new kernel parameter 'cephroot'
or DHCP option 17.
The patch series applies to v3.12-10714-gd085eb6
Thanks
Mark
Mark Doffman (3):
init: Add a new root device option, the Ceph file system
cephroot: Add DHCP option 17 configuration to ceph root fs.
Reuse root_nfs_parse_addr() for NFS and CEPH
Rob Taylor (1):
Documentation: Document the cephroot functionality
Documentation/filesystems/{ => ceph}/ceph.txt | 0
Documentation/filesystems/ceph/cephroot.txt | 85 +++++++++++++
fs/ceph/Kconfig | 10 ++
fs/ceph/Makefile | 1 +
fs/ceph/root.c | 165 ++++++++++++++++++++++++++
fs/nfs/nfsroot.c | 4 +-
include/linux/ceph/ceph_root.h | 10 ++
include/linux/nfs_fs.h | 2 +-
include/linux/root_dev.h | 1 +
init/do_mounts.c | 32 ++++-
net/ipv4/ipconfig.c | 16 ++-
11 files changed, 317 insertions(+), 9 deletions(-)
rename Documentation/filesystems/{ => ceph}/ceph.txt (100%)
create mode 100644 Documentation/filesystems/ceph/cephroot.txt
create mode 100644 fs/ceph/root.c
create mode 100644 include/linux/ceph/ceph_root.h
--
1.8.4
^ permalink raw reply
* [PATCH 1/4] init: Add a new root device option, the Ceph file system
From: mark.doffman @ 2013-11-21 2:13 UTC (permalink / raw)
To: ceph-devel
Cc: Mark Doffman, sage, netdev, linux-kernel, linux-nfs, rob.taylor
In-Reply-To: <1385000024-23463-1-git-send-email-mark.doffman@codethink.co.uk>
From: Mark Doffman <mark.doffman@codethink.co.uk>
Analogous to NFS add a new root device option, the ability
to boot using the Ceph networked file system as the root fs.
This patch adds a new root device option '/dev/ceph' that
uses a ceph networked file system. File system parameters
are passed using a new kernel parameter: 'cephroot'.
The 'cephroot' parameters are very similar to 'nfsroot'.
Signed-off-by: Mark Doffman <mark.doffman@codethink.co.uk>
Reviewed-by: Ian Molton <ian.molton@codethink.co.uk>
---
fs/ceph/Kconfig | 10 +++
fs/ceph/Makefile | 1 +
fs/ceph/root.c | 163 +++++++++++++++++++++++++++++++++++++++++
include/linux/ceph/ceph_root.h | 10 +++
include/linux/root_dev.h | 1 +
init/do_mounts.c | 32 +++++++-
6 files changed, 216 insertions(+), 1 deletion(-)
create mode 100644 fs/ceph/root.c
create mode 100644 include/linux/ceph/ceph_root.h
diff --git a/fs/ceph/Kconfig b/fs/ceph/Kconfig
index ac9a2ef..325e83d 100644
--- a/fs/ceph/Kconfig
+++ b/fs/ceph/Kconfig
@@ -25,3 +25,13 @@ config CEPH_FSCACHE
caching support for Ceph clients using FS-Cache
endif
+
+config ROOT_CEPH
+ bool "Root file system on Ceph FS"
+ depends on CEPH_FS=y && IP_PNP
+ help
+ If you want your system to mount its root file system via CEPH,
+ choose Y here. For details, read
+ <file:Documentation/filesystems/ceph/cephroot.txt>.
+
+ If unsure say N.
diff --git a/fs/ceph/Makefile b/fs/ceph/Makefile
index 32e3010..af2dcbf 100644
--- a/fs/ceph/Makefile
+++ b/fs/ceph/Makefile
@@ -10,3 +10,4 @@ ceph-y := super.o inode.o dir.o file.o locks.o addr.o ioctl.o \
debugfs.o
ceph-$(CONFIG_CEPH_FSCACHE) += cache.o
+ceph-$(CONFIG_ROOT_CEPH) += root.o
diff --git a/fs/ceph/root.c b/fs/ceph/root.c
new file mode 100644
index 0000000..bff67fb
--- /dev/null
+++ b/fs/ceph/root.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (C) 2012 Codethink Ltd. <mark.doffman@codethink.co.uk>
+ *
+ * This file is released under the GPL v2
+ *
+ * Allow a CephFS filesystem to be mounted as root.
+ */
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/string.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/utsname.h>
+#include <linux/root_dev.h>
+#include <linux/in.h>
+#include <net/ipconfig.h>
+#include <linux/ceph/ceph_root.h>
+
+/* linux/net/ipv4/ipconfig.c: trims ip addr off front of name, too. */
+extern __be32 root_nfs_parse_addr(char *name); /*__init*/
+
+#define MAXPATHLEN 1024
+
+/* Parameters passed from the kernel command line */
+static char ceph_root_params[256] __initdata;
+
+/* Address of CEPH server */
+static __be32 servaddr __initdata = htonl(INADDR_NONE);
+
+/* Name of directory to mount */
+static char ceph_export_path[MAXPATHLEN + 1] __initdata;
+
+/* Text-based mount options */
+static char ceph_root_options[256] __initdata;
+
+/* server:path string passed to mount */
+static char ceph_root_device[MAXPATHLEN + 1] __initdata;
+
+/* Address of CEPH server */
+static __be32 root_ceph_server_addr = htonl(INADDR_NONE);
+
+/*
+ * Parse out root export path and mount options from
+ * passed-in string @incoming.
+ *
+ * Copy the export path into @exppath.
+ *
+ * Returns 0 on success -E2BIG if the resulting options string is too long.
+ */
+static int __init root_ceph_parse_options(char *incoming, char *exppath,
+ const size_t exppathlen)
+{
+ char *p;
+ int res = 0;
+
+ /*
+ * Set the remote path
+ */
+ p = strsep(&incoming, ",");
+ if (*p != '\0' && strcmp(p, "default") != 0)
+ strlcpy(exppath, p, exppathlen);
+
+ /*
+ * @incoming now points to the rest of the string; if it
+ * contains something, append it to our root options buffer
+ */
+ if (incoming != NULL && *incoming != '\0') {
+ size_t len = strlen(ceph_root_options);
+ size_t destlen = sizeof(ceph_root_options);
+
+ if (len && ceph_root_options[len - 1] != ',') {
+ if (strlcat(ceph_root_options, ",", destlen) > destlen)
+ res = -E2BIG;
+ }
+
+ if (strlcat(ceph_root_options, incoming, destlen) > destlen)
+ res = -E2BIG;
+
+ }
+ return res;
+}
+
+/*
+ * Parse CephFS server and directory information passed on the kernel
+ * command line.
+ *
+ * cephroot=[<server-ip>:]<root-dir>[,<cephfs-options>]
+ */
+static int __init ceph_root_setup(char *line)
+{
+ ROOT_DEV = Root_CEPH;
+
+ strlcpy(ceph_root_params, line, sizeof(ceph_root_params));
+
+ /*
+ * Note: root_nfs_parse_addr() removes the server-ip from
+ * ceph_root_params, if it exists.
+ */
+ root_ceph_server_addr = root_nfs_parse_addr(ceph_root_params);
+
+ return 1;
+}
+
+__setup("cephroot=", ceph_root_setup);
+
+/*
+ * ceph_root_data - Return mount device and data for CEPHROOT mount.
+ *
+ * @root_device: OUT: Address of string containing CEPHROOT device.
+ * @root_data: OUT: Address of string containing CEPHROOT mount options.
+ *
+ * Returns: 0 and sets @root_device and @root_data if successful.
+ * error code if unsuccessful.
+ */
+int __init ceph_root_data(char **root_device, char **root_data)
+{
+ char *tmp = NULL;
+ const size_t tmplen = sizeof(ceph_export_path);
+ int len;
+ int ret = -E2BIG;
+
+ servaddr = root_ceph_server_addr;
+ if (servaddr == htonl(INADDR_NONE))
+ return -ENOENT;
+
+ tmp = kzalloc(tmplen, GFP_KERNEL);
+ if (tmp == NULL)
+ return -ENOMEM;
+
+ if (ceph_root_params[0] != '\0') {
+ if (root_ceph_parse_options(ceph_root_params, tmp, tmplen))
+ goto out;
+ }
+
+ /*
+ * Set up ceph_root_device. This looks like: server:/path
+ *
+ * At this point, utsname()->nodename contains our local
+ * IP address or hostname, set by ipconfig. If "%s" exists
+ * in tmp, substitute the nodename, then shovel the whole
+ * mess into ceph_root_device.
+ */
+ len = snprintf(ceph_export_path, sizeof(ceph_export_path),
+ tmp, utsname()->nodename);
+ if (len > (int)sizeof(ceph_export_path))
+ goto out;
+ len = snprintf(ceph_root_device, sizeof(ceph_root_device),
+ "%pI4:%s", &servaddr, ceph_export_path);
+ if (len > (int)sizeof(ceph_root_device))
+ goto out;
+
+ pr_debug("Root-CEPH: Root device: %s\n", ceph_root_device);
+ pr_debug("Root-CEPH: Root options: %s\n", ceph_root_options);
+ *root_device = ceph_root_device;
+ *root_data = ceph_root_options;
+
+ ret = 0;
+
+out:
+ kfree(tmp);
+ return ret;
+}
diff --git a/include/linux/ceph/ceph_root.h b/include/linux/ceph/ceph_root.h
new file mode 100644
index 0000000..e6bae63
--- /dev/null
+++ b/include/linux/ceph/ceph_root.h
@@ -0,0 +1,10 @@
+/*
+ * Copyright (C) 2012 Codethink Ltd. <mark.doffman@codethink.co.uk>
+ *
+ * This file is released under the GPL v2
+ *
+ * ceph_root.h
+ */
+
+/* linux/fs/ceph/root.c */
+extern int ceph_root_data(char **root_device, char **root_data); /*__init*/
diff --git a/include/linux/root_dev.h b/include/linux/root_dev.h
index ed241aa..af6b182 100644
--- a/include/linux/root_dev.h
+++ b/include/linux/root_dev.h
@@ -16,6 +16,7 @@ enum {
Root_SDA2 = MKDEV(SCSI_DISK0_MAJOR, 2),
Root_HDC1 = MKDEV(IDE1_MAJOR, 1),
Root_SR0 = MKDEV(SCSI_CDROM_MAJOR, 0),
+ Root_CEPH = MKDEV(UNNAMED_MAJOR, 254),
};
extern dev_t ROOT_DEV;
diff --git a/init/do_mounts.c b/init/do_mounts.c
index 8e5addc..d075020 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -33,6 +33,8 @@
#include <linux/nfs_fs_sb.h>
#include <linux/nfs_mount.h>
+#include <linux/ceph/ceph_root.h>
+
#include "do_mounts.h"
int __initdata rd_doload; /* 1 = load RAM disk, 0 = don't load */
@@ -199,6 +201,7 @@ done:
* a partition with a known unique id.
* 8) <major>:<minor> major and minor number of the device separated by
* a colon.
+ * 9) /dev/ceph represents Root_CEPH
*
* If name doesn't have fall into the categories above, we return (0,0).
* block_class is used to check if something is a disk name. If the disk
@@ -245,7 +248,9 @@ dev_t name_to_dev_t(char *name)
res = Root_RAM0;
if (strcmp(name, "ram") == 0)
goto done;
-
+ res = Root_CEPH;
+ if (strcmp(name, "ceph") == 0)
+ goto done;
if (strlen(name) > 31)
goto fail;
strcpy(s, name);
@@ -473,6 +478,22 @@ static int __init mount_nfs_root(void)
}
#endif
+#ifdef CONFIG_ROOT_CEPH
+static int __init mount_ceph_root(void)
+{
+ char *root_dev, *root_data;
+
+ if (ceph_root_data(&root_dev, &root_data))
+ return 0;
+
+ if (do_mount_root(root_dev, "ceph",
+ root_mountflags, root_data))
+ return 0;
+
+ return 1;
+}
+#endif
+
#if defined(CONFIG_BLK_DEV_RAM) || defined(CONFIG_BLK_DEV_FD)
void __init change_floppy(char *fmt, ...)
{
@@ -514,6 +535,15 @@ void __init mount_root(void)
ROOT_DEV = Root_FD0;
}
#endif
+#ifdef CONFIG_ROOT_CEPH
+ if (ROOT_DEV == Root_CEPH) {
+ if (mount_ceph_root())
+ return;
+
+ printk(KERN_ERR "VFS: Unable to mount root fs via CephFS, trying floppy.\n");
+ ROOT_DEV = Root_FD0;
+ }
+#endif
#ifdef CONFIG_BLK_DEV_FD
if (MAJOR(ROOT_DEV) == FLOPPY_MAJOR) {
/* rd_doload is 2 for a dual initrd/ramload setup */
--
1.8.4
^ permalink raw reply related
* [PATCH 3/4] cephroot: Add DHCP option 17 configuration to ceph root fs.
From: mark.doffman @ 2013-11-21 2:13 UTC (permalink / raw)
To: ceph-devel
Cc: Mark Doffman, sage, netdev, linux-kernel, linux-nfs, rob.taylor
In-Reply-To: <1385000024-23463-1-git-send-email-mark.doffman@codethink.co.uk>
From: Mark Doffman <mark.doffman@codethink.co.uk>
When not configured via kernel parameters add to cephroot
the ability to configure server address, path and options
from DHCP option 17.
Signed-off-by: Mark Doffman <mark.doffman@codethink.co.uk>
Reviewed-by: Ian Molton <ian.molton@codethink.co.uk>
---
Documentation/filesystems/ceph/cephroot.txt | 4 ++++
fs/ceph/root.c | 12 +++++++-----
net/ipv4/ipconfig.c | 10 ++++++++--
3 files changed, 19 insertions(+), 7 deletions(-)
diff --git a/Documentation/filesystems/ceph/cephroot.txt b/Documentation/filesystems/ceph/cephroot.txt
index ae0f5bb..4f12573 100644
--- a/Documentation/filesystems/ceph/cephroot.txt
+++ b/Documentation/filesystems/ceph/cephroot.txt
@@ -49,6 +49,9 @@ This is necessary to enable the pseudo-Ceph-device. Note that it's not a
real device but just a synonym to tell the kernel to use Ceph instead of
a real device.
+If cephroot is not specified, it is expected that that a valid mount will be
+found via DHCP option 17, Root Path [1]
+
cephroot=<monaddr>:/[<subdir>],<ceph-opts>
<monaddr> Monitor address. Each takes the form host[:port]. If the port
@@ -64,6 +67,7 @@ cephroot=<monaddr>:/[<subdir>],<ceph-opts>
4.) References
----------
+[1] http://tools.ietf.org/html/rfc2132
5.) Credits
-------
diff --git a/fs/ceph/root.c b/fs/ceph/root.c
index bff67fb..24b8dcf 100644
--- a/fs/ceph/root.c
+++ b/fs/ceph/root.c
@@ -37,9 +37,6 @@ static char ceph_root_options[256] __initdata;
/* server:path string passed to mount */
static char ceph_root_device[MAXPATHLEN + 1] __initdata;
-/* Address of CEPH server */
-static __be32 root_ceph_server_addr = htonl(INADDR_NONE);
-
/*
* Parse out root export path and mount options from
* passed-in string @incoming.
@@ -97,7 +94,7 @@ static int __init ceph_root_setup(char *line)
* Note: root_nfs_parse_addr() removes the server-ip from
* ceph_root_params, if it exists.
*/
- root_ceph_server_addr = root_nfs_parse_addr(ceph_root_params);
+ root_server_addr = root_nfs_parse_addr(ceph_root_params);
return 1;
}
@@ -120,7 +117,7 @@ int __init ceph_root_data(char **root_device, char **root_data)
int len;
int ret = -E2BIG;
- servaddr = root_ceph_server_addr;
+ servaddr = root_server_addr;
if (servaddr == htonl(INADDR_NONE))
return -ENOENT;
@@ -128,6 +125,11 @@ int __init ceph_root_data(char **root_device, char **root_data)
if (tmp == NULL)
return -ENOMEM;
+ if (root_server_path[0] != '\0') {
+ if (root_ceph_parse_options(root_server_path, tmp, tmplen))
+ goto out;
+ }
+
if (ceph_root_params[0] != '\0') {
if (root_ceph_parse_options(ceph_root_params, tmp, tmplen))
goto out;
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index efa1138..765eea4 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -1435,10 +1435,10 @@ static int __init ip_auto_config(void)
* missing values.
*/
if (ic_myaddr == NONE ||
-#ifdef CONFIG_ROOT_NFS
+#if defined(CONFIG_ROOT_NFS) || defined(CONFIG_ROOT_CEPH)
(root_server_addr == NONE &&
ic_servaddr == NONE &&
- ROOT_DEV == Root_NFS) ||
+ (ROOT_DEV == Root_NFS || ROOT_DEV == Root_CEPH)) ||
#endif
ic_first_dev->next) {
#ifdef IPCONFIG_DYNAMIC
@@ -1465,6 +1465,12 @@ static int __init ip_auto_config(void)
goto try_try_again;
}
#endif
+#ifdef CONFIG_ROOT_CEPH
+ if (ROOT_DEV == Root_CEPH) {
+ pr_err("IP-Config: Retrying forever (CEPH root)...\n");
+ goto try_try_again;
+ }
+#endif
if (--retries) {
pr_err("IP-Config: Reopening network devices...\n");
--
1.8.4
^ permalink raw reply related
* [PATCH 4/4] Reuse root_nfs_parse_addr() for NFS and CEPH
From: mark.doffman @ 2013-11-21 2:13 UTC (permalink / raw)
To: ceph-devel
Cc: Mark Doffman, sage, netdev, linux-kernel, linux-nfs, rob.taylor
In-Reply-To: <1385000024-23463-1-git-send-email-mark.doffman@codethink.co.uk>
From: Mark Doffman <mark.doffman@codethink.co.uk>
Rename function root_nfs_parse_addr to root_parse_server_addr
to reflect its use in parsing an address for both
NFS and CEPH root.
Signed-off-by: Mark Doffman <mark.doffman@codethink.co.uk>
Reviewed-by: Ian Molton <ian.molton@codethink.co.uk>
---
fs/ceph/root.c | 6 +++---
fs/nfs/nfsroot.c | 4 ++--
include/linux/nfs_fs.h | 2 +-
net/ipv4/ipconfig.c | 6 +++---
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/fs/ceph/root.c b/fs/ceph/root.c
index 24b8dcf..17a942f 100644
--- a/fs/ceph/root.c
+++ b/fs/ceph/root.c
@@ -18,7 +18,7 @@
#include <linux/ceph/ceph_root.h>
/* linux/net/ipv4/ipconfig.c: trims ip addr off front of name, too. */
-extern __be32 root_nfs_parse_addr(char *name); /*__init*/
+extern __be32 root_parse_server_addr(char *name); /*__init*/
#define MAXPATHLEN 1024
@@ -91,10 +91,10 @@ static int __init ceph_root_setup(char *line)
strlcpy(ceph_root_params, line, sizeof(ceph_root_params));
/*
- * Note: root_nfs_parse_addr() removes the server-ip from
+ * root_parse_server_addr() removes the server-ip from
* ceph_root_params, if it exists.
*/
- root_server_addr = root_nfs_parse_addr(ceph_root_params);
+ root_server_addr = root_parse_server_addr(ceph_root_params);
return 1;
}
diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c
index cd3c910..4e21716 100644
--- a/fs/nfs/nfsroot.c
+++ b/fs/nfs/nfsroot.c
@@ -144,10 +144,10 @@ static int __init nfs_root_setup(char *line)
* Extract the IP address of the NFS server containing our
* root file system, if one was specified.
*
- * Note: root_nfs_parse_addr() removes the server-ip from
+ * Note: root_parse_server_addr() removes the server-ip from
* nfs_root_parms, if it exists.
*/
- root_server_addr = root_nfs_parse_addr(nfs_root_parms);
+ root_server_addr = root_parse_server_addr(nfs_root_parms);
return 1;
}
diff --git a/include/linux/nfs_fs.h b/include/linux/nfs_fs.h
index 14a4820..0d1fb6e 100644
--- a/include/linux/nfs_fs.h
+++ b/include/linux/nfs_fs.h
@@ -414,7 +414,7 @@ static inline void nfs_display_fhandle(const struct nfs_fh *fh,
*/
extern int nfs_root_data(char **root_device, char **root_data); /*__init*/
/* linux/net/ipv4/ipconfig.c: trims ip addr off front of name, too. */
-extern __be32 root_nfs_parse_addr(char *name); /*__init*/
+extern __be32 root_parse_server_addr(char *name); /*__init*/
/*
* linux/fs/nfs/file.c
diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 765eea4..55a2864 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -140,7 +140,7 @@ __be32 ic_addrservaddr = NONE; /* IP Address of the IP addresses'server */
__be32 ic_servaddr = NONE; /* Boot server IP address */
-__be32 root_server_addr = NONE; /* Address of NFS server */
+__be32 root_server_addr = NONE; /* Address of NFS or CEPH server */
u8 root_server_path[256] = { 0, }; /* Path to mount as root */
__be32 ic_dev_xid; /* Device under configuration */
@@ -1337,7 +1337,7 @@ static const struct file_operations pnp_seq_fops = {
* need to have root_server_addr set _before_ IPConfig gets called as it
* can override it.
*/
-__be32 __init root_nfs_parse_addr(char *name)
+__be32 __init root_parse_server_addr(char *name)
{
__be32 addr;
int octets = 0;
@@ -1491,7 +1491,7 @@ static int __init ip_auto_config(void)
ic_dev = ic_first_dev->dev;
}
- addr = root_nfs_parse_addr(root_server_path);
+ addr = root_parse_server_addr(root_server_path);
if (root_server_addr == NONE)
root_server_addr = addr;
--
1.8.4
^ permalink raw reply related
* [PATCH 2/4] Documentation: Document the cephroot functionality
From: mark.doffman @ 2013-11-21 2:13 UTC (permalink / raw)
To: ceph-devel
Cc: Rob Taylor, sage, netdev, linux-kernel, linux-nfs, Mark Doffman
In-Reply-To: <1385000024-23463-1-git-send-email-mark.doffman@codethink.co.uk>
From: Rob Taylor <rob.taylor@codethink.co.uk>
Document using the cephfs as a root device, its purpose,
functionality and use.
Signed-off-by: Mark Doffman <mark.doffman@codethink.co.uk>
Signed-off-by: Rob Taylor <rob.taylor@codethink.co.uk>
Reviewed-by: Ian Molton <ian.molton@codethink.co.uk>
---
Documentation/filesystems/{ => ceph}/ceph.txt | 0
Documentation/filesystems/ceph/cephroot.txt | 81 +++++++++++++++++++++++++++
2 files changed, 81 insertions(+)
rename Documentation/filesystems/{ => ceph}/ceph.txt (100%)
create mode 100644 Documentation/filesystems/ceph/cephroot.txt
diff --git a/Documentation/filesystems/ceph.txt b/Documentation/filesystems/ceph/ceph.txt
similarity index 100%
rename from Documentation/filesystems/ceph.txt
rename to Documentation/filesystems/ceph/ceph.txt
diff --git a/Documentation/filesystems/ceph/cephroot.txt b/Documentation/filesystems/ceph/cephroot.txt
new file mode 100644
index 0000000..ae0f5bb
--- /dev/null
+++ b/Documentation/filesystems/ceph/cephroot.txt
@@ -0,0 +1,81 @@
+Mounting the root filesystem via Ceph (cephroot)
+===============================================
+
+Written 2013 by Rob Taylor <rob.taylor@codethink.co.uk>
+
+derived from nfsroot.txt:
+
+Written 1996 by Gero Kuhlmann <gero@gkminix.han.de>
+Updated 1997 by Martin Mares <mj@atrey.karlin.mff.cuni.cz>
+Updated 2006 by Nico Schottelius <nico-kernel-nfsroot@schottelius.org>
+Updated 2006 by Horms <horms@verge.net.au>
+
+
+
+In order to use a diskless system, such as an X-terminal or printer server
+for example, it is necessary for the root filesystem to be present on a
+non-disk device. This may be an initramfs (see Documentation/filesystems/
+ramfs-rootfs-initramfs.txt), a ramdisk (see Documentation/initrd.txt), a
+filesystem mounted via NFS or a filesystem mounted via Ceph. The following
+text describes on how to use Ceph for the root filesystem.
+
+For the rest of this text 'client' means the diskless system, and 'server'
+means the Ceph server.
+
+
+1.) Enabling cephroot capabilities
+ -----------------------------
+
+In order to use cephroot, CEPH_FS needs to be selected as
+built-in during configuration. Once this has been selected, the cephroot
+option will become available, which should also be selected.
+
+In the networking options, kernel level autoconfiguration can be selected,
+along with the types of autoconfiguration to support. Selecting all of
+DHCP, BOOTP and RARP is safe.
+
+
+2.) Kernel command line
+ -------------------
+
+When the kernel has been loaded by a boot loader (see below) it needs to be
+told what root fs device to use. And in the case of cephroot, where to find
+both the server and the name of the directory on the server to mount as root.
+This can be established using the following kernel command line parameters:
+
+root=/dev/ceph
+
+This is necessary to enable the pseudo-Ceph-device. Note that it's not a
+real device but just a synonym to tell the kernel to use Ceph instead of
+a real device.
+
+cephroot=<monaddr>:/[<subdir>],<ceph-opts>
+
+ <monaddr> Monitor address. Each takes the form host[:port]. If the port
+ is not specified, the Ceph default of 6789 is assumed.
+
+ <subdir> A subdirectory subdir may be specified if a subset of the file
+ system is to be mounted
+
+ <ceph-opts> Standard Ceph options. All options are separated by commas.
+ See Documentation/filesystems/ceph/ceph.txt for options and
+ their defaults.
+
+4.) References
+ ----------
+
+
+5.) Credits
+ -------
+
+ cephroot was derived from nfsroot by Rob Taylor <rob.taylor@codethink.co.uk>
+ and Mark Doffman <mark.doffman@codethink.co.uk>
+
+ The nfsroot code in the kernel and the RARP support have been written
+ by Gero Kuhlmann <gero@gkminix.han.de>.
+
+ The rest of the IP layer autoconfiguration code has been written
+ by Martin Mares <mj@atrey.karlin.mff.cuni.cz>.
+
+ In order to write the initial version of nfsroot I would like to thank
+ Jens-Uwe Mager <jum@anubis.han.de> for his help.
--
1.8.4
^ permalink raw reply related
* [PATCH v2 1/2] net: rework recvmsg handler msg_name and msg_namelen logic
From: Hannes Frederic Sowa @ 2013-11-21 2:14 UTC (permalink / raw)
To: netdev; +Cc: davem, eric.dumazet, joe
This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
to return msg_name to the user.
This prevents numerous uninitialized memory leaks we had in the
recvmsg handlers and makes it harder for new code to accidentally leak
uninitialized memory.
Optimize for the case recvfrom is called with NULL as address. We don't
need to copy the address at all, so set it to NULL before invoking the
recvmsg handler. We can do so, because all the recvmsg handlers must
cope with the case a plain read() is called on them. read() also sets
msg_name to NULL.
Also document these changes in include/linux/net.h as suggested by David
Miller.
Changes since RFC:
Set msg->msg_name = NULL if user specified a NULL in msg_name but had a
non-null msg_namelen in verify_iovec/verify_compat_iovec. This doesn't
affect sendto as it would bail out earlier while trying to copy-in the
address. It also more naturally reflects the logic by the callers of
verify_iovec.
With this change in place I could remove "
if (!uaddr || msg_sys->msg_namelen == 0)
msg->msg_name = NULL
".
This change does not alter the user visible error logic as we ignore
msg_namelen as long as msg_name is NULL.
Also remove two unnecessary curly brackets in ___sys_recvmsg and change
comments to netdev style.
Cc: David Miller <davem@davemloft.net>
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
v2:
Cleanups suggested by Joe Perches:
a) Remove some instances of unneccessary casts from void* which this patch
touches.
b) Don't set msg_name to NULL if it is already NULL.
Thanks!
As this patch fixes some uninitialized memory leaks I would suggest it for
stable.
crypto/algif_hash.c | 2 --
crypto/algif_skcipher.c | 1 -
drivers/isdn/mISDN/socket.c | 13 ++++---------
drivers/net/ppp/pppoe.c | 2 --
include/linux/net.h | 8 ++++++++
net/appletalk/ddp.c | 16 +++++++---------
net/atm/common.c | 2 --
net/ax25/af_ax25.c | 4 ++--
net/bluetooth/af_bluetooth.c | 9 ++-------
net/bluetooth/hci_sock.c | 2 --
net/bluetooth/rfcomm/sock.c | 1 -
net/bluetooth/sco.c | 1 -
net/caif/caif_socket.c | 4 ----
net/compat.c | 3 ++-
net/core/iovec.c | 3 ++-
net/ipx/af_ipx.c | 3 +--
net/irda/af_irda.c | 4 ----
net/iucv/af_iucv.c | 2 --
net/key/af_key.c | 1 -
net/l2tp/l2tp_ppp.c | 2 --
net/llc/af_llc.c | 2 --
net/netlink/af_netlink.c | 2 --
net/netrom/af_netrom.c | 3 +--
net/nfc/llcp_sock.c | 2 --
net/nfc/rawsock.c | 2 --
net/packet/af_packet.c | 32 +++++++++++++++-----------------
net/rds/recv.c | 2 --
net/rose/af_rose.c | 8 +++++---
net/rxrpc/ar-recvmsg.c | 9 ++++++---
net/socket.c | 19 +++++++++++--------
net/tipc/socket.c | 6 ------
net/unix/af_unix.c | 5 -----
net/vmw_vsock/af_vsock.c | 2 --
net/vmw_vsock/vmci_transport.c | 2 --
net/x25/af_x25.c | 3 +--
35 files changed, 67 insertions(+), 115 deletions(-)
diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 0262210..ef5356c 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -161,8 +161,6 @@ static int hash_recvmsg(struct kiocb *unused, struct socket *sock,
else if (len < ds)
msg->msg_flags |= MSG_TRUNC;
- msg->msg_namelen = 0;
-
lock_sock(sk);
if (ctx->more) {
ctx->more = 0;
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index a1c4f0a..6a6dfc0 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -432,7 +432,6 @@ static int skcipher_recvmsg(struct kiocb *unused, struct socket *sock,
long copied = 0;
lock_sock(sk);
- msg->msg_namelen = 0;
for (iov = msg->msg_iov, iovlen = msg->msg_iovlen; iovlen > 0;
iovlen--, iov++) {
unsigned long seglen = iov->iov_len;
diff --git a/drivers/isdn/mISDN/socket.c b/drivers/isdn/mISDN/socket.c
index e47dcb9..5cefb47 100644
--- a/drivers/isdn/mISDN/socket.c
+++ b/drivers/isdn/mISDN/socket.c
@@ -117,7 +117,6 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
{
struct sk_buff *skb;
struct sock *sk = sock->sk;
- struct sockaddr_mISDN *maddr;
int copied, err;
@@ -135,9 +134,9 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
if (!skb)
return err;
- if (msg->msg_namelen >= sizeof(struct sockaddr_mISDN)) {
- msg->msg_namelen = sizeof(struct sockaddr_mISDN);
- maddr = (struct sockaddr_mISDN *)msg->msg_name;
+ if (msg->msg_name) {
+ struct sockaddr_mISDN *maddr = msg->msg_name;
+
maddr->family = AF_ISDN;
maddr->dev = _pms(sk)->dev->id;
if ((sk->sk_protocol == ISDN_P_LAPD_TE) ||
@@ -150,11 +149,7 @@ mISDN_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
maddr->sapi = _pms(sk)->ch.addr & 0xFF;
maddr->tei = (_pms(sk)->ch.addr >> 8) & 0xFF;
}
- } else {
- if (msg->msg_namelen)
- printk(KERN_WARNING "%s: too small namelen %d\n",
- __func__, msg->msg_namelen);
- msg->msg_namelen = 0;
+ msg->msg_namelen = sizeof(*maddr);
}
copied = skb->len + MISDN_HEADER_LEN;
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 5f66e30..82ee6ed 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -979,8 +979,6 @@ static int pppoe_recvmsg(struct kiocb *iocb, struct socket *sock,
if (error < 0)
goto end;
- m->msg_namelen = 0;
-
if (skb) {
total_len = min_t(size_t, total_len, skb->len);
error = skb_copy_datagram_iovec(skb, 0, m->msg_iov, total_len);
diff --git a/include/linux/net.h b/include/linux/net.h
index b292a04..4bcee94 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -164,6 +164,14 @@ struct proto_ops {
#endif
int (*sendmsg) (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len);
+ /* Notes for implementing recvmsg:
+ * ===============================
+ * msg->msg_namelen should get updated by the recvmsg handlers
+ * iff msg_name != NULL. It is by default 0 to prevent
+ * returning uninitialized memory to user space. The recvfrom
+ * handlers can assume that msg.msg_name is either NULL or has
+ * a minimum size of sizeof(struct sockaddr_storage).
+ */
int (*recvmsg) (struct kiocb *iocb, struct socket *sock,
struct msghdr *m, size_t total_len,
int flags);
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 7fee50d..7d424ac 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1735,7 +1735,6 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
size_t size, int flags)
{
struct sock *sk = sock->sk;
- struct sockaddr_at *sat = (struct sockaddr_at *)msg->msg_name;
struct ddpehdr *ddp;
int copied = 0;
int offset = 0;
@@ -1764,14 +1763,13 @@ static int atalk_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr
}
err = skb_copy_datagram_iovec(skb, offset, msg->msg_iov, copied);
- if (!err) {
- if (sat) {
- sat->sat_family = AF_APPLETALK;
- sat->sat_port = ddp->deh_sport;
- sat->sat_addr.s_node = ddp->deh_snode;
- sat->sat_addr.s_net = ddp->deh_snet;
- }
- msg->msg_namelen = sizeof(*sat);
+ if (!err && msg->msg_name) {
+ struct sockaddr_at *sat = msg->msg_name;
+ sat->sat_family = AF_APPLETALK;
+ sat->sat_port = ddp->deh_sport;
+ sat->sat_addr.s_node = ddp->deh_snode;
+ sat->sat_addr.s_net = ddp->deh_snet;
+ msg->msg_namelen = sizeof(*sat);
}
skb_free_datagram(sk, skb); /* Free the datagram. */
diff --git a/net/atm/common.c b/net/atm/common.c
index 737bef5..7b49100 100644
--- a/net/atm/common.c
+++ b/net/atm/common.c
@@ -531,8 +531,6 @@ int vcc_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
struct sk_buff *skb;
int copied, error = -EINVAL;
- msg->msg_namelen = 0;
-
if (sock->state != SS_CONNECTED)
return -ENOTCONN;
diff --git a/net/ax25/af_ax25.c b/net/ax25/af_ax25.c
index a00123e..7bb1605 100644
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1636,11 +1636,11 @@ static int ax25_recvmsg(struct kiocb *iocb, struct socket *sock,
skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
- if (msg->msg_namelen != 0) {
- struct sockaddr_ax25 *sax = (struct sockaddr_ax25 *)msg->msg_name;
+ if (msg->msg_name) {
ax25_digi digi;
ax25_address src;
const unsigned char *mac = skb_mac_header(skb);
+ struct sockaddr_ax25 *sax = msg->msg_name;
memset(sax, 0, sizeof(struct full_sockaddr_ax25));
ax25_addr_parse(mac + 1, skb->data - mac - 1, &src, NULL,
diff --git a/net/bluetooth/af_bluetooth.c b/net/bluetooth/af_bluetooth.c
index f6a1671..56ca494 100644
--- a/net/bluetooth/af_bluetooth.c
+++ b/net/bluetooth/af_bluetooth.c
@@ -224,10 +224,9 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
skb = skb_recv_datagram(sk, flags, noblock, &err);
if (!skb) {
- if (sk->sk_shutdown & RCV_SHUTDOWN) {
- msg->msg_namelen = 0;
+ if (sk->sk_shutdown & RCV_SHUTDOWN)
return 0;
- }
+
return err;
}
@@ -245,8 +244,6 @@ int bt_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
if (bt_sk(sk)->skb_msg_name)
bt_sk(sk)->skb_msg_name(skb, msg->msg_name,
&msg->msg_namelen);
- else
- msg->msg_namelen = 0;
}
skb_free_datagram(sk, skb);
@@ -295,8 +292,6 @@ int bt_sock_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
if (flags & MSG_OOB)
return -EOPNOTSUPP;
- msg->msg_namelen = 0;
-
BT_DBG("sk %p size %zu", sk, size);
lock_sock(sk);
diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
index 71f0be1..6a6c8bb 100644
--- a/net/bluetooth/hci_sock.c
+++ b/net/bluetooth/hci_sock.c
@@ -856,8 +856,6 @@ static int hci_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
if (!skb)
return err;
- msg->msg_namelen = 0;
-
copied = skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c
index c4d3d42..c80766f 100644
--- a/net/bluetooth/rfcomm/sock.c
+++ b/net/bluetooth/rfcomm/sock.c
@@ -615,7 +615,6 @@ static int rfcomm_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
if (test_and_clear_bit(RFCOMM_DEFER_SETUP, &d->flags)) {
rfcomm_dlc_accept(d);
- msg->msg_namelen = 0;
return 0;
}
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 12a0e51..24fa396 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -711,7 +711,6 @@ static int sco_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
test_bit(BT_SK_DEFER_SETUP, &bt_sk(sk)->flags)) {
sco_conn_defer_accept(pi->conn->hcon, pi->setting);
sk->sk_state = BT_CONFIG;
- msg->msg_namelen = 0;
release_sock(sk);
return 0;
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 05a41c7..d6be3ed 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -286,8 +286,6 @@ static int caif_seqpkt_recvmsg(struct kiocb *iocb, struct socket *sock,
if (m->msg_flags&MSG_OOB)
goto read_error;
- m->msg_namelen = 0;
-
skb = skb_recv_datagram(sk, flags, 0 , &ret);
if (!skb)
goto read_error;
@@ -361,8 +359,6 @@ static int caif_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
if (flags&MSG_OOB)
goto out;
- msg->msg_namelen = 0;
-
/*
* Lock the socket to prevent queue disordering
* while sleeps in memcpy_tomsg
diff --git a/net/compat.c b/net/compat.c
index 8903258..618c6a8 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -93,7 +93,8 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
if (err < 0)
return err;
}
- kern_msg->msg_name = kern_address;
+ if (kern_msg->msg_name)
+ kern_msg->msg_name = kern_address;
} else
kern_msg->msg_name = NULL;
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 4cdb7c4..b618694 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -48,7 +48,8 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
if (err < 0)
return err;
}
- m->msg_name = address;
+ if (m->msg_name)
+ m->msg_name = address;
} else {
m->msg_name = NULL;
}
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index 7a1e0fc..e096025 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1823,8 +1823,6 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
if (skb->tstamp.tv64)
sk->sk_stamp = skb->tstamp;
- msg->msg_namelen = sizeof(*sipx);
-
if (sipx) {
sipx->sipx_family = AF_IPX;
sipx->sipx_port = ipx->ipx_source.sock;
@@ -1832,6 +1830,7 @@ static int ipx_recvmsg(struct kiocb *iocb, struct socket *sock,
sipx->sipx_network = IPX_SKB_CB(skb)->ipx_source_net;
sipx->sipx_type = ipx->ipx_type;
sipx->sipx_zero = 0;
+ msg->msg_namelen = sizeof(*sipx);
}
rc = copied;
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index 0f67690..de7db23 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -1385,8 +1385,6 @@ static int irda_recvmsg_dgram(struct kiocb *iocb, struct socket *sock,
IRDA_DEBUG(4, "%s()\n", __func__);
- msg->msg_namelen = 0;
-
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
if (!skb)
@@ -1451,8 +1449,6 @@ static int irda_recvmsg_stream(struct kiocb *iocb, struct socket *sock,
target = sock_rcvlowat(sk, flags & MSG_WAITALL, size);
timeo = sock_rcvtimeo(sk, noblock);
- msg->msg_namelen = 0;
-
do {
int chunk;
struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);
diff --git a/net/iucv/af_iucv.c b/net/iucv/af_iucv.c
index 168aff5..c4b7218 100644
--- a/net/iucv/af_iucv.c
+++ b/net/iucv/af_iucv.c
@@ -1324,8 +1324,6 @@ static int iucv_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
int err = 0;
u32 offset;
- msg->msg_namelen = 0;
-
if ((sk->sk_state == IUCV_DISCONN) &&
skb_queue_empty(&iucv->backlog_skb_q) &&
skb_queue_empty(&sk->sk_receive_queue) &&
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 911ef03..545f047 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -3616,7 +3616,6 @@ static int pfkey_recvmsg(struct kiocb *kiocb,
if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
goto out;
- msg->msg_namelen = 0;
skb = skb_recv_datagram(sk, flags, flags & MSG_DONTWAIT, &err);
if (skb == NULL)
goto out;
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index ffda81e..be5fadf 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -197,8 +197,6 @@ static int pppol2tp_recvmsg(struct kiocb *iocb, struct socket *sock,
if (sk->sk_state & PPPOX_BOUND)
goto end;
- msg->msg_namelen = 0;
-
err = 0;
skb = skb_recv_datagram(sk, flags & ~MSG_DONTWAIT,
flags & MSG_DONTWAIT, &err);
diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index 6cba486..7b01b9f 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -720,8 +720,6 @@ static int llc_ui_recvmsg(struct kiocb *iocb, struct socket *sock,
int target; /* Read at least this many bytes */
long timeo;
- msg->msg_namelen = 0;
-
lock_sock(sk);
copied = -ENOTCONN;
if (unlikely(sk->sk_type == SOCK_STREAM && sk->sk_state == TCP_LISTEN))
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index f0176e1..bca50b9 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2335,8 +2335,6 @@ static int netlink_recvmsg(struct kiocb *kiocb, struct socket *sock,
}
#endif
- msg->msg_namelen = 0;
-
copied = data_skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index 698814b..53c19a3 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -1179,10 +1179,9 @@ static int nr_recvmsg(struct kiocb *iocb, struct socket *sock,
sax->sax25_family = AF_NETROM;
skb_copy_from_linear_data_offset(skb, 7, sax->sax25_call.ax25_call,
AX25_ADDR_LEN);
+ msg->msg_namelen = sizeof(*sax);
}
- msg->msg_namelen = sizeof(*sax);
-
skb_free_datagram(sk, skb);
release_sock(sk);
diff --git a/net/nfc/llcp_sock.c b/net/nfc/llcp_sock.c
index d308402..824c605 100644
--- a/net/nfc/llcp_sock.c
+++ b/net/nfc/llcp_sock.c
@@ -807,8 +807,6 @@ static int llcp_sock_recvmsg(struct kiocb *iocb, struct socket *sock,
pr_debug("%p %zu\n", sk, len);
- msg->msg_namelen = 0;
-
lock_sock(sk);
if (sk->sk_state == LLCP_CLOSED &&
diff --git a/net/nfc/rawsock.c b/net/nfc/rawsock.c
index cd958b3..66bcd2e 100644
--- a/net/nfc/rawsock.c
+++ b/net/nfc/rawsock.c
@@ -244,8 +244,6 @@ static int rawsock_recvmsg(struct kiocb *iocb, struct socket *sock,
if (!skb)
return rc;
- msg->msg_namelen = 0;
-
copied = skb->len;
if (len < copied) {
msg->msg_flags |= MSG_TRUNC;
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2e8286b..61bd50a 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2660,7 +2660,6 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
struct sock *sk = sock->sk;
struct sk_buff *skb;
int copied, err;
- struct sockaddr_ll *sll;
int vnet_hdr_len = 0;
err = -EINVAL;
@@ -2744,22 +2743,10 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
goto out_free;
}
- /*
- * If the address length field is there to be filled in, we fill
- * it in now.
- */
-
- sll = &PACKET_SKB_CB(skb)->sa.ll;
- if (sock->type == SOCK_PACKET)
- msg->msg_namelen = sizeof(struct sockaddr_pkt);
- else
- msg->msg_namelen = sll->sll_halen + offsetof(struct sockaddr_ll, sll_addr);
-
- /*
- * You lose any data beyond the buffer you gave. If it worries a
- * user program they can ask the device for its MTU anyway.
+ /* You lose any data beyond the buffer you gave. If it worries
+ * a user program they can ask the device for its MTU
+ * anyway.
*/
-
copied = skb->len;
if (copied > len) {
copied = len;
@@ -2772,9 +2759,20 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
sock_recv_ts_and_drops(msg, sk, skb);
- if (msg->msg_name)
+ if (msg->msg_name) {
+ /* If the address length field is there to be filled
+ * in, we fill it in now.
+ */
+ if (sock->type == SOCK_PACKET) {
+ msg->msg_namelen = sizeof(struct sockaddr_pkt);
+ } else {
+ struct sockaddr_ll *sll = &PACKET_SKB_CB(skb)->sa.ll;
+ msg->msg_namelen = sll->sll_halen +
+ offsetof(struct sockaddr_ll, sll_addr);
+ }
memcpy(msg->msg_name, &PACKET_SKB_CB(skb)->sa,
msg->msg_namelen);
+ }
if (pkt_sk(sk)->auxdata) {
struct tpacket_auxdata aux;
diff --git a/net/rds/recv.c b/net/rds/recv.c
index 9f0f17c..de339b2 100644
--- a/net/rds/recv.c
+++ b/net/rds/recv.c
@@ -410,8 +410,6 @@ int rds_recvmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
rdsdebug("size %zu flags 0x%x timeo %ld\n", size, msg_flags, timeo);
- msg->msg_namelen = 0;
-
if (msg_flags & MSG_OOB)
goto out;
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index e98fcfb..33af772 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -1216,7 +1216,6 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock,
{
struct sock *sk = sock->sk;
struct rose_sock *rose = rose_sk(sk);
- struct sockaddr_rose *srose = (struct sockaddr_rose *)msg->msg_name;
size_t copied;
unsigned char *asmptr;
struct sk_buff *skb;
@@ -1252,8 +1251,11 @@ static int rose_recvmsg(struct kiocb *iocb, struct socket *sock,
skb_copy_datagram_iovec(skb, 0, msg->msg_iov, copied);
- if (srose != NULL) {
- memset(srose, 0, msg->msg_namelen);
+ if (msg->msg_name) {
+ struct sockaddr_rose *srose;
+
+ memset(msg->msg_name, 0, sizeof(struct full_sockaddr_rose));
+ srose = msg->msg_name;
srose->srose_family = AF_ROSE;
srose->srose_addr = rose->dest_addr;
srose->srose_call = rose->dest_call;
diff --git a/net/rxrpc/ar-recvmsg.c b/net/rxrpc/ar-recvmsg.c
index 4b48687..898492a 100644
--- a/net/rxrpc/ar-recvmsg.c
+++ b/net/rxrpc/ar-recvmsg.c
@@ -143,10 +143,13 @@ int rxrpc_recvmsg(struct kiocb *iocb, struct socket *sock,
/* copy the peer address and timestamp */
if (!continue_call) {
- if (msg->msg_name && msg->msg_namelen > 0)
+ if (msg->msg_name) {
+ size_t len =
+ sizeof(call->conn->trans->peer->srx);
memcpy(msg->msg_name,
- &call->conn->trans->peer->srx,
- sizeof(call->conn->trans->peer->srx));
+ &call->conn->trans->peer->srx, len);
+ msg->msg_namelen = len;
+ }
sock_recv_ts_and_drops(msg, &rx->sk, skb);
}
diff --git a/net/socket.c b/net/socket.c
index c226ace..fc28556 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1840,8 +1840,10 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
msg.msg_iov = &iov;
iov.iov_len = size;
iov.iov_base = ubuf;
- msg.msg_name = (struct sockaddr *)&address;
- msg.msg_namelen = sizeof(address);
+ /* Save some cycles and don't copy the address if not needed */
+ msg.msg_name = addr ? (struct sockaddr *)&address : NULL;
+ /* We assume all kernel code knows the size of sockaddr_storage */
+ msg.msg_namelen = 0;
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
err = sock_recvmsg(sock, &msg, size, flags);
@@ -2221,16 +2223,14 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
goto out;
}
- /*
- * Save the user-mode address (verify_iovec will change the
- * kernel msghdr to use the kernel address space)
+ /* Save the user-mode address (verify_iovec will change the
+ * kernel msghdr to use the kernel address space)
*/
-
uaddr = (__force void __user *)msg_sys->msg_name;
uaddr_len = COMPAT_NAMELEN(msg);
- if (MSG_CMSG_COMPAT & flags) {
+ if (MSG_CMSG_COMPAT & flags)
err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
- } else
+ else
err = verify_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
if (err < 0)
goto out_freeiov;
@@ -2239,6 +2239,9 @@ static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
cmsg_ptr = (unsigned long)msg_sys->msg_control;
msg_sys->msg_flags = flags & (MSG_CMSG_CLOEXEC|MSG_CMSG_COMPAT);
+ /* We assume all kernel code knows the size of sockaddr_storage */
+ msg_sys->msg_namelen = 0;
+
if (sock->file->f_flags & O_NONBLOCK)
flags |= MSG_DONTWAIT;
err = (nosec ? sock_recvmsg_nosec : sock_recvmsg)(sock, msg_sys,
diff --git a/net/tipc/socket.c b/net/tipc/socket.c
index 3906527..3b61851 100644
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -980,9 +980,6 @@ static int recv_msg(struct kiocb *iocb, struct socket *sock,
goto exit;
}
- /* will be updated in set_orig_addr() if needed */
- m->msg_namelen = 0;
-
timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
restart:
@@ -1091,9 +1088,6 @@ static int recv_stream(struct kiocb *iocb, struct socket *sock,
goto exit;
}
- /* will be updated in set_orig_addr() if needed */
- m->msg_namelen = 0;
-
target = sock_rcvlowat(sk, flags & MSG_WAITALL, buf_len);
timeout = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index c1f403b..01625cc 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1754,7 +1754,6 @@ static void unix_copy_addr(struct msghdr *msg, struct sock *sk)
{
struct unix_sock *u = unix_sk(sk);
- msg->msg_namelen = 0;
if (u->addr) {
msg->msg_namelen = u->addr->len;
memcpy(msg->msg_name, u->addr->name, u->addr->len);
@@ -1778,8 +1777,6 @@ static int unix_dgram_recvmsg(struct kiocb *iocb, struct socket *sock,
if (flags&MSG_OOB)
goto out;
- msg->msg_namelen = 0;
-
err = mutex_lock_interruptible(&u->readlock);
if (err) {
err = sock_intr_errno(sock_rcvtimeo(sk, noblock));
@@ -1924,8 +1921,6 @@ static int unix_stream_recvmsg(struct kiocb *iocb, struct socket *sock,
target = sock_rcvlowat(sk, flags&MSG_WAITALL, size);
timeo = sock_rcvtimeo(sk, flags&MSG_DONTWAIT);
- msg->msg_namelen = 0;
-
/* Lock the socket to prevent queue disordering
* while sleeps in memcpy_tomsg
*/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 545c08b..5adfd94 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1662,8 +1662,6 @@ vsock_stream_recvmsg(struct kiocb *kiocb,
vsk = vsock_sk(sk);
err = 0;
- msg->msg_namelen = 0;
-
lock_sock(sk);
if (sk->sk_state != SS_CONNECTED) {
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 9d69866..687360d 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -1746,8 +1746,6 @@ static int vmci_transport_dgram_dequeue(struct kiocb *kiocb,
if (flags & MSG_OOB || flags & MSG_ERRQUEUE)
return -EOPNOTSUPP;
- msg->msg_namelen = 0;
-
/* Retrieve the head sk_buff from the socket's receive queue. */
err = 0;
skb = skb_recv_datagram(&vsk->sk, flags, noblock, &err);
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index 45a3ab5..7622789 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1340,10 +1340,9 @@ static int x25_recvmsg(struct kiocb *iocb, struct socket *sock,
if (sx25) {
sx25->sx25_family = AF_X25;
sx25->sx25_addr = x25->dest_addr;
+ msg->msg_namelen = sizeof(*sx25);
}
- msg->msg_namelen = sizeof(struct sockaddr_x25);
-
x25_check_rbuf(sk);
rc = copied;
out_free_dgram:
--
1.8.3.1
^ permalink raw reply related
* [PATCH 2/2] net: add BUG_ON if kernel advertises msg_namelen > sizeof(struct sockaddr_storage)
From: Hannes Frederic Sowa @ 2013-11-21 2:14 UTC (permalink / raw)
To: netdev; +Cc: eric.dumazet
In that case it is probable that kernel code overwrote part of the
stack. So we should bail out loudly here.
The BUG_ON may be removed in future if we are sure all protocols are
conformant.
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
In the long term I want to add BUILD_BUG_ONs to catch these. I currently don't
know how to add them so they don't get missed by future new implementations.
Maybe a coccinelle check is better suited for this job.
I would suggest it for stable, too. Maybe after it cooked a bit on net
and in a RC.
net/socket.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/socket.c b/net/socket.c
index fc28556..0b18693 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -221,12 +221,13 @@ static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
int err;
int len;
+ BUG_ON(klen > sizeof(struct sockaddr_storage));
err = get_user(len, ulen);
if (err)
return err;
if (len > klen)
len = klen;
- if (len < 0 || len > sizeof(struct sockaddr_storage))
+ if (len < 0)
return -EINVAL;
if (len) {
if (audit_sockaddr(klen, kaddr))
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] bonding: If IP route look-up to send an ARP fails, mark in bonding structure as no ARP sent.
From: rama nichanamatlu @ 2013-11-21 2:23 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev
In-Reply-To: <12668.1384996699@death.nxdomain>
On 11/20/2013 5:18 PM, Jay Vosburgh wrote:
> rama nichanamatlu <rama.nichanamatlu@oracle.com> wrote:
>
>> During the creation of VLAN's atop bonding the underlying interfaces are
>> made part of VLAN's, and at the same bonding driver gets aware that VLAN's
>> exists above it and hence would consult IP routing for every ARP to be
>> sent to determine the route which tells bonding driver the correct VLAN
>> tag to attach to the outgoing ARP packet. But, during the VLAN creation
>> when vlan driver puts the underlying interface into default vlan and then
>> actual vlan, in-between this if bonding driver consults the IP for a
>> route, IP fails to provide a correct route and upon which bonding driver
>> drops the ARP packet. ARP monitor when it
>> comes around next time, sees no ARP response and fails-over to the next
>> available slave. Consulting for a IP route, ip_route_output(),happens in
>> bond_arp_send_all().
>>
>> To prevent this false fail-over, when bonding driver fails to send an ARP
>> out it marks in its private structure, bonding{}, not to expect an ARP
>> response, when ARP monitor comes around next time ARP sending will be
>> tried again.
>>
>> Extensively tested in a VM environment; sr-iov intf->bonding intf->vlan
>> intf. All virtual interfaces created at boot time.
>
> First, this patch appears to be for an older kernel, as the
> current mainline code is substantially different (e.g., master_ip is no
> longer used).
>
> Second, won't this methodology mask legitimate failures, such as
> when a single arp_ip_target specifies a destination that is not ever
> reachable? I.e., would specifying a permanently unreachable IP address
> as the arp_ip_target cause all slaves to always stay up (because no ARPs
> will ever be sent), even if no ARP replies are ever received?
>
> -J
>
Thank U.
I agree with your rationale. Would keep a slave falsely up but traffic
might flow. And true that, it is not what we are looking for.
We can try a different approach too, which we used to fix a false
fail-over in MTU changing case where the device interface takes time to
change the device MTU. And in the mean time bonding was failing over.
What we did to fix was to stop the ARP monitoring, bond_change_mtu(),
and restart it when NETDEV_CHANGE from slave is handled,
bond_slave_netdev_event(). Not sure if this can used for vlan case, as
mtu thing is event driven.
>> Orabug: 17172660
>> Signed-off-by: Venkat Venkatsubra <venkat.x.venkatsubra@oracle.com>
>> Signed-off-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com>
^ permalink raw reply
* Re: Get rxhash fixes and RFS support in tun
From: David Miller @ 2013-11-21 2:23 UTC (permalink / raw)
To: therbert; +Cc: netdev, edumazet, hkchu
In-Reply-To: <CA+mtBx91hSmYKrbUsPrtnXRjYR8QDGZ+GAAnMixauUqw9LFkiQ@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 20 Nov 2013 17:09:01 -0800
> I think the sw_rxhash holds more useful information. In
> skb_get_rxhash() either we have an L4 hash or we try to get one by
> doing the SW computation, this means we don't ever return a HW hash
> which is not L4 so basically that case is treated as an invalid hash.
> So after a call to skb_get_rxhash, at least one of l4_rxhash or
> sw_rxhash is and we won't redo flow dissection on subsequent calls
> unless the rxhash is cleared.
Ok.... one interesting issue remains, which is that the SW flow
dissector considers decapsulating protocols like AH and ESP as
"L4".
I think we'll need to do something about that at some point.
^ permalink raw reply
* Re: Get rxhash fixes and RFS support in tun
From: Tom Herbert @ 2013-11-21 2:42 UTC (permalink / raw)
To: David Miller; +Cc: Linux Netdev List, Eric Dumazet, Jerry Chu
In-Reply-To: <20131120.212321.700376819925646003.davem@davemloft.net>
On Wed, Nov 20, 2013 at 6:23 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Wed, 20 Nov 2013 17:09:01 -0800
>
>> I think the sw_rxhash holds more useful information. In
>> skb_get_rxhash() either we have an L4 hash or we try to get one by
>> doing the SW computation, this means we don't ever return a HW hash
>> which is not L4 so basically that case is treated as an invalid hash.
>> So after a call to skb_get_rxhash, at least one of l4_rxhash or
>> sw_rxhash is and we won't redo flow dissection on subsequent calls
>> unless the rxhash is cleared.
>
> Ok.... one interesting issue remains, which is that the SW flow
> dissector considers decapsulating protocols like AH and ESP as
> "L4".
>
Thinking about a little more, maybe the valid bit is better. Using
rxhash for both populating the flow hash table for consumption at a
low level and using it for flow lookup at a higher level might be at
odds. In the original patches Eric mentioned that tun should always
compute its own hash anyway.
> I think we'll need to do something about that at some point.
We need the rxhash to be the value seen at the point of RPS (to do RFS
correctly), which I think probably means we don't ever want to change
it after the first calculation! (clearing at tunnel decap wouldn't be
correct either) For ESP or AH, I believe it's appropriate to use SPI
as a substitute for ports.
Thanks for the comments,
Tom
^ permalink raw reply
* Re: Get rxhash fixes and RFS support in tun
From: David Miller @ 2013-11-21 2:50 UTC (permalink / raw)
To: therbert; +Cc: netdev, edumazet, hkchu
In-Reply-To: <CA+mtBx8g9uYOc0euQiEDi8qfx_ZggsE_Wn8N5R+peE1_N4WKEg@mail.gmail.com>
From: Tom Herbert <therbert@google.com>
Date: Wed, 20 Nov 2013 18:42:48 -0800
> We need the rxhash to be the value seen at the point of RPS (to do RFS
> correctly), which I think probably means we don't ever want to change
> it after the first calculation! (clearing at tunnel decap wouldn't be
> correct either) For ESP or AH, I believe it's appropriate to use SPI
> as a substitute for ports.
But that means that all connections going over the same IPSEC path
hash to the same value.
I really think that xfrm_input() should zap the rxhash near the
existing nf_reset() call.
The same argument goes for tunnels, that is why ip_tunnel_core.c does
what it does with the rxhash clearing right now.
In both the IPSEC tunnel (not transport) and normal IP/GRE tunnel
cases, it's a completely new SKB receive, done via netif_rx(), after
decapsulation.
That leaves only IPSEC transport mode as the only case where RFS isn't
(re-)performed but we can build infrastructure to make that happen.
^ permalink raw reply
* Re: [PATCH v2 1/2] net: rework recvmsg handler msg_name and msg_namelen logic
From: David Miller @ 2013-11-21 3:06 UTC (permalink / raw)
To: hannes; +Cc: netdev, eric.dumazet, joe
In-Reply-To: <20131121021422.GA934@order.stressinduktion.org>
From: Hannes Frederic Sowa <hannes@stressinduktion.org>
Date: Thu, 21 Nov 2013 03:14:22 +0100
> This patch now always passes msg->msg_namelen as 0. recvmsg handlers must
> set msg_namelen to the proper size <= sizeof(struct sockaddr_storage)
> to return msg_name to the user.
These two patches look fantastic, applied and queued up for -stable
thanks!
^ permalink raw reply
* Re: [PATCH v3 1/4] net/phy: Add VSC8234 support
From: David Miller @ 2013-11-21 3:10 UTC (permalink / raw)
To: Shruti; +Cc: netdev, afleming, galak
In-Reply-To: <1384987099-4518-1-git-send-email-Shruti@freescale.com>
From: <Shruti@freescale.com>
Date: Wed, 20 Nov 2013 16:38:16 -0600
> From: Andy Fleming <afleming@gmail.com>
>
> Vitesse VSC8234 is quad port 10/100/1000BASE-T PHY
> with SGMII and SERDES MAC interfaces.
>
> Signed-off-by: Andy Fleming <afleming@gmail.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> Signed-off-by: Shruti Kanetkar <Shruti@freescale.com>
> ---
> drivers/net/phy/vitesse.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> v3: No content change. v1 & 2 didn't get picked up by Patchwork.
> No idea what else to do so trying to re-submit from a different
> machine (git version, etc.)
These didn't make it to patchwork again, I wonder if it has to do
with the gratuitous angle brackets around "<netdev@vger.kernel.org>"
in your headers.
Whatever the reason I lack the time to look into the cause, so I've
just applied these patches.
Thanks.
^ permalink raw reply
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