* [PATCH net] xfrm: Release dst if this dst is improper for vti tunnel
From: Fan Du @ 2013-11-19 8:53 UTC (permalink / raw)
To: steffen.klassert, saurabh.mohan; +Cc: davem, netdev
After searching rt by the vti tunnel dst/src parameter,
if this rt has neither attached to any transformation
nor the transformation is not tunnel oriented, this rt
should be released back to ip layer.
otherwise causing dst memory leakage.
Signed-off-by: Fan Du <fan.du@windriver.com>
---
net/ipv4/ip_vti.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 5d9c845..52b802a 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -126,6 +126,7 @@ static netdev_tx_t vti_tunnel_xmit(struct sk_buff *skb, struct net_device *dev)
if (!rt->dst.xfrm ||
rt->dst.xfrm->props.mode != XFRM_MODE_TUNNEL) {
dev->stats.tx_carrier_errors++;
+ ip_rt_put(rt);
goto tx_error_icmp;
}
tdev = rt->dst.dev;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH RFC 0/9]net: stmmac PM related fixes.
From: srinivas kandagatla @ 2013-11-19 9:03 UTC (permalink / raw)
To: Giuseppe CAVALLARO; +Cc: netdev, linux-kernel
In-Reply-To: <528AF617.70802@st.com>
On 19/11/13 05:24, Giuseppe CAVALLARO wrote:
>
> So you continue to have my Acked-by for all.
>
Thanks for the Ack Peppe.
> peppe
^ permalink raw reply
* Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec pkt
From: Fan Du @ 2013-11-19 9:16 UTC (permalink / raw)
To: Saurabh Mohan
Cc: Christophe Gouault, Steffen Klassert, David S. Miller, Herbert Xu,
netdev@vger.kernel.org, Sergei Shtylyov, Eric Dumazet
In-Reply-To: <1902752B0C92F943AB7EA9EE13E2DEEC1273BA977C@HQ1-EXCH02.corp.brocade.com>
Hi, Saurabh
On 2013年11月19日 05:38, Saurabh Mohan wrote:
>
>
>> -----Original Message-----
>> From: Christophe Gouault [mailto:christophe.gouault@6wind.com]
>> Sent: Thursday, November 07, 2013 4:56 AM
>> To: Steffen Klassert
>> Cc: David S. Miller; Herbert Xu; netdev@vger.kernel.org; Saurabh Mohan;
>> Sergei Shtylyov; Eric Dumazet
>> Subject: Re: [PATCH net v3] vti: fix spd lookup: match plaintext pkt, not ipsec
>> pkt
>>
>> Hello Steffen,
>>
>> I am also interested in knowing Saurabh's intentions regarding the
>> behavior of policies bound to vti interfaces.
>>
> The semantics is to match the policy "src 0.0.0.0/0 dst 0.0.0.0/0 proto any"
> That is the only policy that VTI should use. The mark is needed to
> distinguish and limit the policy to a specific vti tunnel interface only.
> There is no other policy that may be applied to a vti interface.
> The fact that traffic is going over the tunnel interface implies that it
> must be encrypted/decrypted. Applying the above policy is a way
> to achieve that.
I'm not much experienced with VTI usage practical production usage scenario, but
I have one question about the necessity of policy checking on VTI receiving part.
- A VTI tunnel is hashed by destination address and i_key when creating them;
- After each tunneled IP packet delivered to vti_rcv, the first step is looking
for the right tunnel, this is done by using tunneled IP packet outer source and
destination address without any key matching rule involved.
If there are any other tunnel with the same source/destination address, but not
the same mark in place, the tunnel lookup in the vti_rcv will properly not hit
VTI tunnel, but the non-VTI tunnel. So the VTI net device statistics will not be
accurate, and what's the point of checking policy for the wrong tunnel interface?
Or the VTI tunnel is the only tunnel with this specific source/destination address
in the production deployment. Again the upper layer 4 will check the policy after
all, that's the right place to do the policy checking.
So IMO, it's unnecessary to check policy for a net_device like VTI, actually I hold
a patch of removing the VTI policy checking due to net-next closure for the moment.
>> However, please note that setting a policy with a wildcard selector
>> works in both cases (before or after this patch), so a common test case
>> can be defined.
>>
>> Actually the *previous* patch on vti (7263a5187f9e vti: get rid of nf
>> mark rule in prerouting) introduced significant changes, and implies
>> behaviors dependant on the kernel version, but it seemed to meet
>> Saurabh's agreement, as the following thread witnesses:
>>
>> http://www.spinics.net/lists/netdev/msg253134.html
>>
> Getting rid of the pre-routing mark, which had to be done outside of
> the vti tunnel code was prone to misconfiguration.
> Though it is unfortunate that it creates a kernel version dependency.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
浮沉随浪只记今朝笑
--fan fan
^ permalink raw reply
* [PATCH] netlink: fix documentation typo in netlink_set_err()
From: Johannes Berg @ 2013-11-19 9:35 UTC (permalink / raw)
To: netdev; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
The parameter is just 'group', not 'groups', fix the documentation typo.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/netlink/af_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index 8df7f64..f0176e1 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2017,7 +2017,7 @@ out:
* netlink_set_err - report error to broadcast listeners
* @ssk: the kernel netlink socket, as returned by netlink_kernel_create()
* @portid: the PORTID of a process that we want to skip (if any)
- * @groups: the broadcast group that will notice the error
+ * @group: the broadcast group that will notice the error
* @code: error code, must be negative (as usual in kernelspace)
*
* This function returns the number of broadcast listeners that have set the
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH net v3 1/2] net: sched: tbf: fix calculation of max_size
From: Jesper Dangaard Brouer @ 2013-11-19 9:38 UTC (permalink / raw)
To: Yang Yingliang; +Cc: davem, netdev, eric.dumazet, brouer, jpirko
In-Reply-To: <1384845939-8424-2-git-send-email-yangyingliang@huawei.com>
On Tue, 19 Nov 2013 15:25:38 +0800
Yang Yingliang <yangyingliang@huawei.com> wrote:
> commit b757c9336d63f94c6b57532(tbf: improved accuracy at high rates)
> introduce a regression.
>
> With the follow command:
> tc qdisc add dev eth1 root handle 1: tbf latency 50ms burst 10KB rate 30gbit mtu 64k
>
> Without this patch, the max_size value is 10751(bytes).
> But, in fact, the real max_size value should be smaller than 7440(bytes).
> Or a packet whose length is bigger than 7440 will cause network congestion.
> Because the packet is so big that can't get enough tokens. Even all the tokens
> in the buffer is given to the packet.
Sorry, but I don't like the commit message. The real problem is the
value in q->buffer, and that the userspace rate table cannot handle
these high rates, which you don't mention.
I would write something like:
The kernel no longer uses the userspace provided rate table. Thus, it
is wrong to calculate max_size based on this rate table. At high rates
this rate table gets very inaccurate, which can lead wrong calculation
of max_size.
Consequence of max_size being too large is severe, and cause packets
being stalled in tbf_dequeue() because it cannot get enough tokens.
The max_size guards against enqueuing packet sizes above q->buffer
"time" in tbf_enqueue().
This patch fixes the calculation of max_size. By ... add desc ...
perhaps also mention how it is connected to p->mtu (with is also a
"time" value).
The rest of the patch looks okay now, one point below though.
> diff --git a/net/sched/sch_tbf.c b/net/sched/sch_tbf.c
> index 68f9859..c194129 100644
> --- a/net/sched/sch_tbf.c
> +++ b/net/sched/sch_tbf.c
[...]
> @@ -339,30 +326,46 @@ static int tbf_change(struct Qdisc *sch, struct nlattr *opt)
[...]
> + for (n = 0; n < 65536; n++)
> + if (psched_l2t_ns(&q->rate, n) > q->buffer)
> + break;
> + max_size = min_t(u32, n, (256ULL << qopt->rate.cell_log) - 1);
I'm a little uncertain about, if using the 65536 constant is okay, or
considered "bad style".
I'm still a little confused/uncertain why we need the "qopt->rate.cell_log".
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Sr. Network Kernel Developer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* RE: net/usb/ax88179_178a driver broken in linux-3.12
From: David Laight @ 2013-11-19 10:04 UTC (permalink / raw)
To: Mark Lord, Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <528A9A36.50903@pobox.com>
> From: Mark Lord
> On 13-11-18 08:32 AM, David Laight wrote:
> >> From: David Laight
> >>> On 13-11-17 01:56 PM, Mark Lord wrote:
> >>>> On 13-11-17 01:35 PM, Mark Lord wrote:
> >>>>> The USB3 network adapter locks up consistently for me here in 3.12,
> >>>>> but was working without issues in 3.11.x
> >>
> >> The xhci driver is well broken in 3.12.
> >
> > To correct myself...
> >
> > The xhci driver has never correctly support scatter-gather requests.
> > In 3.12 code was added to usbnet to generate SG transmits, and to the
> > ax88179_178a driver to use them.
> > TCP segmentation offload was then enabled - with does generate
> > SG transfers.
> >
> > SG transfers for 'disks' almost certainly work because the
> > fragment boundaries are 'adequately aligned'.
> >
> > David
> >
>
> Well, that's all very nice and whatnot,
> except the ax88179_178a driver still does not work in linux-3.12,
> whereas it works fine in all earlier kernels.
>
> That's a regression.
> And a simple revert (earlier in this thread) fixes it.
>
> So.. let's revert it for now, until a proper xhci compatible patch is produced.
Which changes did you revert?
I think you must have gone right back to the version that doesn't so scatter-gather.
There is a patch to xhci-ring.c that should fix the SG problem.
http://www.spinics.net/lists/linux-usb/msg97176.html
I think it should apply to the 3.12 sources.
Note that with older kernels the asix card I have will drop/discard
short bursts of tx/rx packets. I've not seen that with 3.12 and the
xhci-ring.c patch.
David
^ permalink raw reply
* [PATCH] atm: idt77252: fix dev refcnt leak
From: Ying Xue @ 2013-11-19 10:09 UTC (permalink / raw)
To: chas; +Cc: linux-atm-general, netdev, linux-kernel, ying.xue
init_card() calls dev_get_by_name() to get a network deceive. But it
doesn't decrease network device reference count after the device is
used.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
---
drivers/atm/idt77252.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/atm/idt77252.c b/drivers/atm/idt77252.c
index 272f009..1bdf104 100644
--- a/drivers/atm/idt77252.c
+++ b/drivers/atm/idt77252.c
@@ -3511,7 +3511,7 @@ static int init_card(struct atm_dev *dev)
tmp = dev_get_by_name(&init_net, tname); /* jhs: was "tmp = dev_get(tname);" */
if (tmp) {
memcpy(card->atmdev->esi, tmp->dev_addr, 6);
-
+ dev_put(tmp);
printk("%s: ESI %pM\n", card->name, card->atmdev->esi);
}
/*
--
1.7.9.5
^ permalink raw reply related
* [patch net-next RFC v2 0/2] ipv6: allow temporary address management for user-created addresses
From: Jiri Pirko @ 2013-11-19 10:43 UTC (permalink / raw)
To: netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw, David.Laight
Both kernel patches were modified, see details in each patch description.
No modification to iproute2 patches was done.
Jiri Pirko (2):
ipv6 addrconf: extend ifa_flags to u32
ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage
temporary addresses
include/net/addrconf.h | 4 +-
include/net/if_inet6.h | 2 +-
include/uapi/linux/if_addr.h | 5 ++
net/ipv6/addrconf.c | 197 +++++++++++++++++++++++++------------------
4 files changed, 122 insertions(+), 86 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [patch net-next RFC v2 1/2] ipv6 addrconf: extend ifa_flags to u32
From: Jiri Pirko @ 2013-11-19 10:43 UTC (permalink / raw)
To: netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw, David.Laight
In-Reply-To: <1384857809-6573-1-git-send-email-jiri@resnulli.us>
There is no more space in u8 ifa_flags. So do what davem suffested and
add another netlink attr called IFA_FLAGS for carry more flags.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
v1->v2:
- reordered struct inet6_ifaddr as suggested by David Laight
- changed flags type to u32 on couple of places as suggested by Thomas Haller
- changed output of if6_seq_show as suggested by Thomas Haller
include/net/addrconf.h | 4 ++--
include/net/if_inet6.h | 2 +-
include/uapi/linux/if_addr.h | 4 ++++
net/ipv6/addrconf.c | 32 ++++++++++++++++++++------------
4 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 86505bf..e70278e 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -81,9 +81,9 @@ int ipv6_dev_get_saddr(struct net *net, const struct net_device *dev,
const struct in6_addr *daddr, unsigned int srcprefs,
struct in6_addr *saddr);
int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
- unsigned char banned_flags);
+ u32 banned_flags);
int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
- unsigned char banned_flags);
+ u32 banned_flags);
int ipv6_rcv_saddr_equal(const struct sock *sk, const struct sock *sk2);
void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr);
void addrconf_leave_solict(struct inet6_dev *idev, const struct in6_addr *addr);
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index 76d5427..b58c36c 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -50,8 +50,8 @@ struct inet6_ifaddr {
int state;
+ __u32 flags;
__u8 dad_probes;
- __u8 flags;
__u16 scope;
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index 23357ab..8ab0c2c 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -18,6 +18,9 @@ struct ifaddrmsg {
* It makes no difference for normally configured broadcast interfaces,
* but for point-to-point IFA_ADDRESS is DESTINATION address,
* local address is supplied in IFA_LOCAL attribute.
+ *
+ * IFA_FLAGS is a u32 attribute that extends the u8 field ifa_flags.
+ * If present, the value from struct ifaddrmsg will be ignored.
*/
enum {
IFA_UNSPEC,
@@ -28,6 +31,7 @@ enum {
IFA_ANYCAST,
IFA_CACHEINFO,
IFA_MULTICAST,
+ IFA_FLAGS,
__IFA_MAX,
};
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 542d095..c94da31 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1393,7 +1393,7 @@ try_nextdev:
EXPORT_SYMBOL(ipv6_dev_get_saddr);
int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
- unsigned char banned_flags)
+ u32 banned_flags)
{
struct inet6_ifaddr *ifp;
int err = -EADDRNOTAVAIL;
@@ -1410,7 +1410,7 @@ int __ipv6_get_lladdr(struct inet6_dev *idev, struct in6_addr *addr,
}
int ipv6_get_lladdr(struct net_device *dev, struct in6_addr *addr,
- unsigned char banned_flags)
+ u32 banned_flags)
{
struct inet6_dev *idev;
int err = -EADDRNOTAVAIL;
@@ -2181,7 +2181,7 @@ ok:
}
if (ifp) {
- int flags;
+ u32 flags;
unsigned long now;
struct inet6_ifaddr *ift;
u32 stored_lft;
@@ -2366,10 +2366,11 @@ err_exit:
/*
* Manual configuration of address on an interface
*/
-static int inet6_addr_add(struct net *net, int ifindex, const struct in6_addr *pfx,
+static int inet6_addr_add(struct net *net, int ifindex,
+ const struct in6_addr *pfx,
const struct in6_addr *peer_pfx,
- unsigned int plen, __u8 ifa_flags, __u32 prefered_lft,
- __u32 valid_lft)
+ unsigned int plen, __u32 ifa_flags,
+ __u32 prefered_lft, __u32 valid_lft)
{
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
@@ -3357,7 +3358,7 @@ static void if6_seq_stop(struct seq_file *seq, void *v)
static int if6_seq_show(struct seq_file *seq, void *v)
{
struct inet6_ifaddr *ifp = (struct inet6_ifaddr *)v;
- seq_printf(seq, "%pi6 %02x %02x %02x %02x %8s\n",
+ seq_printf(seq, "%pi6 %02x %02x %02x %03x %8s\n",
&ifp->addr,
ifp->idev->dev->ifindex,
ifp->prefix_len,
@@ -3578,6 +3579,7 @@ static const struct nla_policy ifa_ipv6_policy[IFA_MAX+1] = {
[IFA_ADDRESS] = { .len = sizeof(struct in6_addr) },
[IFA_LOCAL] = { .len = sizeof(struct in6_addr) },
[IFA_CACHEINFO] = { .len = sizeof(struct ifa_cacheinfo) },
+ [IFA_FLAGS] = { .len = sizeof(u32) },
};
static int
@@ -3601,7 +3603,7 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh)
return inet6_addr_del(net, ifm->ifa_index, pfx, ifm->ifa_prefixlen);
}
-static int inet6_addr_modify(struct inet6_ifaddr *ifp, u8 ifa_flags,
+static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
u32 prefered_lft, u32 valid_lft)
{
u32 flags;
@@ -3656,7 +3658,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
struct inet6_ifaddr *ifa;
struct net_device *dev;
u32 valid_lft = INFINITY_LIFE_TIME, preferred_lft = INFINITY_LIFE_TIME;
- u8 ifa_flags;
+ u32 ifa_flags;
int err;
err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv6_policy);
@@ -3683,8 +3685,10 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
if (dev == NULL)
return -ENODEV;
+ ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
+
/* We ignore other flags so far. */
- ifa_flags = ifm->ifa_flags & (IFA_F_NODAD | IFA_F_HOMEADDRESS);
+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS;
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
@@ -3708,7 +3712,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
return err;
}
-static void put_ifaddrmsg(struct nlmsghdr *nlh, u8 prefixlen, u8 flags,
+static void put_ifaddrmsg(struct nlmsghdr *nlh, u8 prefixlen, u32 flags,
u8 scope, int ifindex)
{
struct ifaddrmsg *ifm;
@@ -3751,7 +3755,8 @@ static inline int inet6_ifaddr_msgsize(void)
return NLMSG_ALIGN(sizeof(struct ifaddrmsg))
+ nla_total_size(16) /* IFA_LOCAL */
+ nla_total_size(16) /* IFA_ADDRESS */
- + nla_total_size(sizeof(struct ifa_cacheinfo));
+ + nla_total_size(sizeof(struct ifa_cacheinfo))
+ + nla_total_size(4) /* IFA_FLAGS */;
}
static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
@@ -3799,6 +3804,9 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
if (put_cacheinfo(skb, ifa->cstamp, ifa->tstamp, preferred, valid) < 0)
goto error;
+ if (nla_put_u32(skb, IFA_FLAGS, ifa->flags) < 0)
+ goto error;
+
return nlmsg_end(skb, nlh);
error:
--
1.8.3.1
^ permalink raw reply related
* [patch net-next RFC v2 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses
From: Jiri Pirko @ 2013-11-19 10:43 UTC (permalink / raw)
To: netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw, David.Laight
In-Reply-To: <1384857809-6573-1-git-send-email-jiri@resnulli.us>
Creating an address with this flag set will result in kernel taking care
of temporary addresses in the same way as if the address was created by
kernel itself (after RA receive). This allows userspace applications
implementing the autoconfiguration (NetworkManager for example) to
implement ipv6 addresses privacy.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
v1->v2:
- Added possibility to change IFA_F_MANAGETEMPADDR flag as suggested by Thomas Haller
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 167 ++++++++++++++++++++++++-------------------
2 files changed, 96 insertions(+), 72 deletions(-)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index 8ab0c2c..cfed10b 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -48,6 +48,7 @@ enum {
#define IFA_F_DEPRECATED 0x20
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
+#define IFA_F_MANAGETEMPADDR 0x100
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index c94da31..a29e782 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1010,7 +1010,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
u32 addr_flags;
unsigned long now = jiffies;
- write_lock(&idev->lock);
+ write_lock_bh(&idev->lock);
if (ift) {
spin_lock_bh(&ift->lock);
memcpy(&addr.s6_addr[8], &ift->addr.s6_addr[8], 8);
@@ -1022,7 +1022,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
retry:
in6_dev_hold(idev);
if (idev->cnf.use_tempaddr <= 0) {
- write_unlock(&idev->lock);
+ write_unlock_bh(&idev->lock);
pr_info("%s: use_tempaddr is disabled\n", __func__);
in6_dev_put(idev);
ret = -1;
@@ -1032,7 +1032,7 @@ retry:
if (ifp->regen_count++ >= idev->cnf.regen_max_retry) {
idev->cnf.use_tempaddr = -1; /*XXX*/
spin_unlock_bh(&ifp->lock);
- write_unlock(&idev->lock);
+ write_unlock_bh(&idev->lock);
pr_warn("%s: regeneration time exceeded - disabled temporary address support\n",
__func__);
in6_dev_put(idev);
@@ -1058,7 +1058,7 @@ retry:
regen_advance = idev->cnf.regen_max_retry *
idev->cnf.dad_transmits *
idev->nd_parms->retrans_time / HZ;
- write_unlock(&idev->lock);
+ write_unlock_bh(&idev->lock);
/* A temporary address is created only if this calculated Preferred
* Lifetime is greater than REGEN_ADVANCE time units. In particular,
@@ -1085,7 +1085,7 @@ retry:
in6_dev_put(idev);
pr_info("%s: retry temporary address regeneration\n", __func__);
tmpaddr = &addr;
- write_lock(&idev->lock);
+ write_lock_bh(&idev->lock);
goto retry;
}
@@ -2019,6 +2019,73 @@ static struct inet6_dev *addrconf_add_dev(struct net_device *dev)
return idev;
}
+static void manage_tempaddrs(struct inet6_dev *idev,
+ struct inet6_ifaddr *ifp,
+ __u32 valid_lft, __u32 prefered_lft,
+ bool create, unsigned long now)
+{
+ u32 flags;
+ struct inet6_ifaddr *ift;
+
+ read_lock_bh(&idev->lock);
+ /* update all temporary addresses in the list */
+ list_for_each_entry(ift, &idev->tempaddr_list, tmp_list) {
+ int age, max_valid, max_prefered;
+
+ if (ifp != ift->ifpub)
+ continue;
+
+ /* RFC 4941 section 3.3:
+ * If a received option will extend the lifetime of a public
+ * address, the lifetimes of temporary addresses should
+ * be extended, subject to the overall constraint that no
+ * temporary addresses should ever remain "valid" or "preferred"
+ * for a time longer than (TEMP_VALID_LIFETIME) or
+ * (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR), respectively.
+ */
+ age = (now - ift->cstamp) / HZ;
+ max_valid = idev->cnf.temp_valid_lft - age;
+ if (max_valid < 0)
+ max_valid = 0;
+
+ max_prefered = idev->cnf.temp_prefered_lft -
+ idev->cnf.max_desync_factor - age;
+ if (max_prefered < 0)
+ max_prefered = 0;
+
+ if (valid_lft > max_valid)
+ valid_lft = max_valid;
+
+ if (prefered_lft > max_prefered)
+ prefered_lft = max_prefered;
+
+ spin_lock(&ift->lock);
+ flags = ift->flags;
+ ift->valid_lft = valid_lft;
+ ift->prefered_lft = prefered_lft;
+ ift->tstamp = now;
+ if (prefered_lft > 0)
+ ift->flags &= ~IFA_F_DEPRECATED;
+
+ spin_unlock(&ift->lock);
+ if (!(flags&IFA_F_TENTATIVE))
+ ipv6_ifa_notify(0, ift);
+ }
+
+ if ((create || list_empty(&idev->tempaddr_list)) &&
+ idev->cnf.use_tempaddr > 0) {
+ /* When a new public address is created as described
+ * in [ADDRCONF], also create a new temporary address.
+ * Also create a temporary address if it's enabled but
+ * no temporary address currently exists.
+ */
+ read_unlock_bh(&idev->lock);
+ ipv6_create_tempaddr(ifp, NULL);
+ } else {
+ read_unlock_bh(&idev->lock);
+ }
+}
+
void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
{
struct prefix_info *pinfo;
@@ -2173,6 +2240,7 @@ ok:
return;
}
+ ifp->flags |= IFA_F_MANAGETEMPADDR;
update_lft = 0;
create = 1;
ifp->cstamp = jiffies;
@@ -2183,7 +2251,6 @@ ok:
if (ifp) {
u32 flags;
unsigned long now;
- struct inet6_ifaddr *ift;
u32 stored_lft;
/* update lifetime (RFC2462 5.5.3 e) */
@@ -2224,70 +2291,8 @@ ok:
} else
spin_unlock(&ifp->lock);
- read_lock_bh(&in6_dev->lock);
- /* update all temporary addresses in the list */
- list_for_each_entry(ift, &in6_dev->tempaddr_list,
- tmp_list) {
- int age, max_valid, max_prefered;
-
- if (ifp != ift->ifpub)
- continue;
-
- /*
- * RFC 4941 section 3.3:
- * If a received option will extend the lifetime
- * of a public address, the lifetimes of
- * temporary addresses should be extended,
- * subject to the overall constraint that no
- * temporary addresses should ever remain
- * "valid" or "preferred" for a time longer than
- * (TEMP_VALID_LIFETIME) or
- * (TEMP_PREFERRED_LIFETIME - DESYNC_FACTOR),
- * respectively.
- */
- age = (now - ift->cstamp) / HZ;
- max_valid = in6_dev->cnf.temp_valid_lft - age;
- if (max_valid < 0)
- max_valid = 0;
-
- max_prefered = in6_dev->cnf.temp_prefered_lft -
- in6_dev->cnf.max_desync_factor -
- age;
- if (max_prefered < 0)
- max_prefered = 0;
-
- if (valid_lft > max_valid)
- valid_lft = max_valid;
-
- if (prefered_lft > max_prefered)
- prefered_lft = max_prefered;
-
- spin_lock(&ift->lock);
- flags = ift->flags;
- ift->valid_lft = valid_lft;
- ift->prefered_lft = prefered_lft;
- ift->tstamp = now;
- if (prefered_lft > 0)
- ift->flags &= ~IFA_F_DEPRECATED;
-
- spin_unlock(&ift->lock);
- if (!(flags&IFA_F_TENTATIVE))
- ipv6_ifa_notify(0, ift);
- }
-
- if ((create || list_empty(&in6_dev->tempaddr_list)) && in6_dev->cnf.use_tempaddr > 0) {
- /*
- * When a new public address is created as
- * described in [ADDRCONF], also create a new
- * temporary address. Also create a temporary
- * address if it's enabled but no temporary
- * address currently exists.
- */
- read_unlock_bh(&in6_dev->lock);
- ipv6_create_tempaddr(ifp, NULL);
- } else {
- read_unlock_bh(&in6_dev->lock);
- }
+ manage_tempaddrs(in6_dev, ifp, valid_lft, prefered_lft,
+ create, now);
in6_ifa_put(ifp);
addrconf_verify(0);
@@ -2429,6 +2434,9 @@ static int inet6_addr_add(struct net *net, int ifindex,
* manually configured addresses
*/
addrconf_dad_start(ifp);
+ if (ifa_flags & IFA_F_MANAGETEMPADDR)
+ manage_tempaddrs(idev, ifp, valid_lft, prefered_lft,
+ true, jiffies);
in6_ifa_put(ifp);
addrconf_verify(0);
return 0;
@@ -3609,6 +3617,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
u32 flags;
clock_t expires;
unsigned long timeout;
+ bool was_managetempaddr;
if (!valid_lft || (prefered_lft > valid_lft))
return -EINVAL;
@@ -3632,7 +3641,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
}
spin_lock_bh(&ifp->lock);
- ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
+ was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+ ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
+ IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+ ifp->flags |= ifa_flags;
ifp->tstamp = jiffies;
ifp->valid_lft = valid_lft;
ifp->prefered_lft = prefered_lft;
@@ -3643,6 +3655,14 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
expires, flags);
+
+ if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
+ if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
+ valid_lft = prefered_lft = 0;
+ manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
+ false, jiffies);
+ }
+
addrconf_verify(0);
return 0;
@@ -3687,8 +3707,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
+ if (ifa_flags & IFA_F_MANAGETEMPADDR && ifm->ifa_prefixlen != 64)
+ return -EINVAL;
+
/* We ignore other flags so far. */
- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS;
+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR;
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
--
1.8.3.1
^ permalink raw reply related
* [patch iproute2 RFC v2 1/2] add support for extended ifa_flags
From: Jiri Pirko @ 2013-11-19 10:44 UTC (permalink / raw)
To: netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw, David.Laight
In-Reply-To: <1384857809-6573-1-git-send-email-jiri@resnulli.us>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
include/linux/if_addr.h | 1 +
ip/ipaddress.c | 44 ++++++++++++++++++++++++++++++--------------
2 files changed, 31 insertions(+), 14 deletions(-)
diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index 58b39f4..cced59f 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -28,6 +28,7 @@ enum {
IFA_ANYCAST,
IFA_CACHEINFO,
IFA_MULTICAST,
+ IFA_FLAGS,
__IFA_MAX,
};
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 1c3e4da..59dbd71 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -541,6 +541,13 @@ static int set_lifetime(unsigned int *lifetime, char *argv)
return 0;
}
+static unsigned int get_ifa_flags(struct ifaddrmsg *ifa,
+ struct rtattr *ifa_flags_attr)
+{
+ return ifa_flags_attr ? rta_getattr_u32(ifa_flags_attr) :
+ ifa->ifa_flags;
+}
+
int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
void *arg)
{
@@ -567,6 +574,8 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
parse_rtattr(rta_tb, IFA_MAX, IFA_RTA(ifa), n->nlmsg_len - NLMSG_LENGTH(sizeof(*ifa)));
+ ifa_flags = get_ifa_flags(ifa, rta_tb[IFA_FLAGS]);
+
if (!rta_tb[IFA_LOCAL])
rta_tb[IFA_LOCAL] = rta_tb[IFA_ADDRESS];
if (!rta_tb[IFA_ADDRESS])
@@ -576,7 +585,7 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
return 0;
if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
return 0;
- if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
+ if ((filter.flags ^ ifa_flags) & filter.flagmask)
return 0;
if (filter.label) {
SPRINT_BUF(b1);
@@ -670,36 +679,35 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
abuf, sizeof(abuf)));
}
fprintf(fp, "scope %s ", rtnl_rtscope_n2a(ifa->ifa_scope, b1, sizeof(b1)));
- ifa_flags = ifa->ifa_flags;
- if (ifa->ifa_flags&IFA_F_SECONDARY) {
+ if (ifa_flags & IFA_F_SECONDARY) {
ifa_flags &= ~IFA_F_SECONDARY;
if (ifa->ifa_family == AF_INET6)
fprintf(fp, "temporary ");
else
fprintf(fp, "secondary ");
}
- if (ifa->ifa_flags&IFA_F_TENTATIVE) {
+ if (ifa_flags & IFA_F_TENTATIVE) {
ifa_flags &= ~IFA_F_TENTATIVE;
fprintf(fp, "tentative ");
}
- if (ifa->ifa_flags&IFA_F_DEPRECATED) {
+ if (ifa_flags & IFA_F_DEPRECATED) {
ifa_flags &= ~IFA_F_DEPRECATED;
deprecated = 1;
fprintf(fp, "deprecated ");
}
- if (ifa->ifa_flags&IFA_F_HOMEADDRESS) {
+ if (ifa_flags & IFA_F_HOMEADDRESS) {
ifa_flags &= ~IFA_F_HOMEADDRESS;
fprintf(fp, "home ");
}
- if (ifa->ifa_flags&IFA_F_NODAD) {
+ if (ifa_flags & IFA_F_NODAD) {
ifa_flags &= ~IFA_F_NODAD;
fprintf(fp, "nodad ");
}
- if (!(ifa->ifa_flags&IFA_F_PERMANENT)) {
+ if (!(ifa_flags & IFA_F_PERMANENT)) {
fprintf(fp, "dynamic ");
} else
ifa_flags &= ~IFA_F_PERMANENT;
- if (ifa->ifa_flags&IFA_F_DADFAILED) {
+ if (ifa_flags & IFA_F_DADFAILED) {
ifa_flags &= ~IFA_F_DADFAILED;
fprintf(fp, "dadfailed ");
}
@@ -926,6 +934,8 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
for (a = ainfo->head; a; a = a->next) {
struct nlmsghdr *n = &a->h;
struct ifaddrmsg *ifa = NLMSG_DATA(n);
+ struct rtattr *tb[IFA_MAX + 1];
+ unsigned int ifa_flags;
if (ifa->ifa_index != ifi->ifi_index)
continue;
@@ -934,11 +944,13 @@ static void ipaddr_filter(struct nlmsg_chain *linfo, struct nlmsg_chain *ainfo)
continue;
if ((filter.scope^ifa->ifa_scope)&filter.scopemask)
continue;
- if ((filter.flags^ifa->ifa_flags)&filter.flagmask)
+
+ parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
+ ifa_flags = get_ifa_flags(ifa, tb[IFA_FLAGS]);
+
+ if ((filter.flags ^ ifa_flags) & filter.flagmask)
continue;
if (filter.pfx.family || filter.label) {
- struct rtattr *tb[IFA_MAX+1];
- parse_rtattr(tb, IFA_MAX, IFA_RTA(ifa), IFA_PAYLOAD(n));
if (!tb[IFA_LOCAL])
tb[IFA_LOCAL] = tb[IFA_ADDRESS];
@@ -1252,6 +1264,7 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
__u32 preferred_lft = INFINITY_LIFE_TIME;
__u32 valid_lft = INFINITY_LIFE_TIME;
struct ifa_cacheinfo cinfo;
+ unsigned int ifa_flags = 0;
memset(&req, 0, sizeof(req));
@@ -1329,9 +1342,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
if (set_lifetime(&preferred_lft, *argv))
invarg("preferred_lft value", *argv);
} else if (strcmp(*argv, "home") == 0) {
- req.ifa.ifa_flags |= IFA_F_HOMEADDRESS;
+ ifa_flags |= IFA_F_HOMEADDRESS;
} else if (strcmp(*argv, "nodad") == 0) {
- req.ifa.ifa_flags |= IFA_F_NODAD;
+ ifa_flags |= IFA_F_NODAD;
} else {
if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
@@ -1349,6 +1362,9 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
}
argc--; argv++;
}
+ req.ifa.ifa_flags = ifa_flags & 0xff;
+ addattr32(&req.n, sizeof(req), IFA_FLAGS, ifa_flags);
+
if (d == NULL) {
fprintf(stderr, "Not enough information: \"dev\" argument is required.\n");
return -1;
--
1.8.3.1
^ permalink raw reply related
* [patch iproute2 RFC v2 2/2] add support for IFA_F_MANAGETEMPADDR
From: Jiri Pirko @ 2013-11-19 10:44 UTC (permalink / raw)
To: netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw, David.Laight
In-Reply-To: <1384857859-6618-1-git-send-email-jiri@resnulli.us>
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
include/linux/if_addr.h | 1 +
ip/ipaddress.c | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/include/linux/if_addr.h b/include/linux/if_addr.h
index cced59f..e1e95ce 100644
--- a/include/linux/if_addr.h
+++ b/include/linux/if_addr.h
@@ -45,6 +45,7 @@ enum {
#define IFA_F_DEPRECATED 0x20
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
+#define IFA_F_MANAGETEMPADDR 0x0100
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 59dbd71..fa6417d 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -703,6 +703,10 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
ifa_flags &= ~IFA_F_NODAD;
fprintf(fp, "nodad ");
}
+ if (ifa_flags & IFA_F_MANAGETEMPADDR) {
+ ifa_flags &= ~IFA_F_MANAGETEMPADDR;
+ fprintf(fp, "mngtmpaddr ");
+ }
if (!(ifa_flags & IFA_F_PERMANENT)) {
fprintf(fp, "dynamic ");
} else
@@ -1345,6 +1349,8 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
ifa_flags |= IFA_F_HOMEADDRESS;
} else if (strcmp(*argv, "nodad") == 0) {
ifa_flags |= IFA_F_NODAD;
+ } else if (strcmp(*argv, "mngtmpaddr") == 0) {
+ ifa_flags |= IFA_F_MANAGETEMPADDR;
} else {
if (strcmp(*argv, "local") == 0) {
NEXT_ARG();
--
1.8.3.1
^ permalink raw reply related
* [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
From: Daniel Borkmann @ 2013-11-19 10:51 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp, Wang Weidong, Eric Dumazet
Wang reported an issue that lksctp's test_getname_v6 seems to fail.
The issue is that we do not copy sk_v6_rcv_saddr over to the new
socket, although the comment above says so regarding rcv_saddr.
Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
correctly.") originally moved that over to sctp_copy_sock(), but
after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
this no longer holds and the actual value of sk_v6_rcv_saddr was
no longer being migrated.
With this patch, the lksctp test suite passes again for IPv6.
Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
---
net/sctp/ipv6.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 7567e6f..be08592 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
* and getpeername().
*/
sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
+ newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
sk_refcnt_debug_inc(newsk);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 2/2] ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
From: Daniel Borkmann @ 2013-11-19 11:04 UTC (permalink / raw)
To: wangweidong
Cc: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman, dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <528ADBDD.9040209@huawei.com>
On 11/19/2013 04:32 AM, wangweidong wrote:
> On 2013/11/19 11:14, Hannes Frederic Sowa wrote:
>> On Tue, Nov 19, 2013 at 10:47:27AM +0800, Wang Weidong wrote:
>>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>>> index 0740f93..83d011e 100644
>>> --- a/net/ipv6/tcp_ipv6.c
>>> +++ b/net/ipv6/tcp_ipv6.c
>>> @@ -1116,6 +1116,10 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
>>> newtp = tcp_sk(newsk);
>>>
>>> memcpy(newnp, np, sizeof(struct ipv6_pinfo));
>>> + /* Don't forget copy the rcv_saddr and daddr when
>>> + * copy ipv6_pinfo.
>>> + */
>>> + sk_v6_copy_addrs(newsk, sk);
>>>
>>> ipv6_addr_set_v4mapped(newinet->inet_daddr, &newsk->sk_v6_daddr);
>>>
>>
>> Hmm, how did you spot this?
>>
>> Greetings,
>>
>> Hannes
>>
>
> When I did the lksctp-tools(1.0.15)/src/func_tests/test_getname_v6, I got a Segmentation fault.
> So I try to resolve it. I found the sctp_accept will call sctp_v6_create_accept_sk in IPV6,
> the function will memcpy the ipv6_pinfo, and not copy the sk_v6_rcv_saddr. But the getsockname
> will read the sk_v6_rcv_saddr, and the value is not true.
In SCTP code, "newsk->sk_v6_daddr = oldsk->sk_v6_daddr;" is redundant as it's already
done by "sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);" for the primary path,
after we're doing the full copy of struct ipv6_pinfo to the new socket in
sctp_v6_create_accept_sk(). I've sent out an updated patch for SCTP.
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix copying sk_v6_rcv_saddr in sctp_v6_create_accept_sk
From: wangweidong @ 2013-11-19 11:08 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: netdev, linux-sctp, Eric Dumazet
In-Reply-To: <1384858284-2246-1-git-send-email-dborkman@redhat.com>
On 2013/11/19 18:51, Daniel Borkmann wrote:
> Wang reported an issue that lksctp's test_getname_v6 seems to fail.
>
> The issue is that we do not copy sk_v6_rcv_saddr over to the new
> socket, although the comment above says so regarding rcv_saddr.
>
> Commit 914e1c8b6980 ("sctp: Inherit all socket options from parent
> correctly.") originally moved that over to sctp_copy_sock(), but
> after commit efe4208f47f9 ("ipv6: make lookups simpler and faster")
> this no longer holds and the actual value of sk_v6_rcv_saddr was
> no longer being migrated.
>
> With this patch, the lksctp test suite passes again for IPv6.
>
> Fixes: efe4208f47f9 ("ipv6: make lookups simpler and faster")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Signed-off-by: Wang Weidong <wangweidong1@huawei.com>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> net/sctp/ipv6.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 7567e6f..be08592 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -661,6 +661,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
> * and getpeername().
> */
> sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);
> + newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> sk_refcnt_debug_inc(newsk);
>
Acked-by: Wang Weidong <wangweidong1@huawei.com>
>
^ permalink raw reply
* Re: [PATCH 2/2] ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
From: wangweidong @ 2013-11-19 11:09 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem, gerrit, kuznet, jmorris, yoshfuji, kaber, vyasevich,
nhorman, dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <528B45BA.8030808@redhat.com>
On 2013/11/19 19:04, Daniel Borkmann wrote:
> On 11/19/2013 04:32 AM, wangweidong wrote:
>> On 2013/11/19 11:14, Hannes Frederic Sowa wrote:
>>> On Tue, Nov 19, 2013 at 10:47:27AM +0800, Wang Weidong wrote:
>>>> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
>>>> index 0740f93..83d011e 100644
>>>> --- a/net/ipv6/tcp_ipv6.c
>>>> +++ b/net/ipv6/tcp_ipv6.c
>>>> @@ -1116,6 +1116,10 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
>>>> newtp = tcp_sk(newsk);
>>>>
>>>> memcpy(newnp, np, sizeof(struct ipv6_pinfo));
>>>> + /* Don't forget copy the rcv_saddr and daddr when
>>>> + * copy ipv6_pinfo.
>>>> + */
>>>> + sk_v6_copy_addrs(newsk, sk);
>>>>
>>>> ipv6_addr_set_v4mapped(newinet->inet_daddr, &newsk->sk_v6_daddr);
>>>>
>>>
>>> Hmm, how did you spot this?
>>>
>>> Greetings,
>>>
>>> Hannes
>>>
>>
>> When I did the lksctp-tools(1.0.15)/src/func_tests/test_getname_v6, I got a Segmentation fault.
>> So I try to resolve it. I found the sctp_accept will call sctp_v6_create_accept_sk in IPV6,
>> the function will memcpy the ipv6_pinfo, and not copy the sk_v6_rcv_saddr. But the getsockname
>> will read the sk_v6_rcv_saddr, and the value is not true.
>
> In SCTP code, "newsk->sk_v6_daddr = oldsk->sk_v6_daddr;" is redundant as it's already
> done by "sctp_v6_to_sk_daddr(&asoc->peer.primary_addr, newsk);" for the primary path,
> after we're doing the full copy of struct ipv6_pinfo to the new socket in
> sctp_v6_create_accept_sk(). I've sent out an updated patch for SCTP.
>
Yeah, I had seen it. And I Acked-by it.
Thanks.
> .
>
^ permalink raw reply
* Re: ipv6 fragmentation-related panic in netfilter
From: Wolfgang Walter @ 2013-11-19 11:11 UTC (permalink / raw)
To: Patrick McHardy; +Cc: Steffen Klassert, Tomas Hlavacek, netdev, netfilter-devel
In-Reply-To: <20131101092536.GA17499@macbook.localnet>
Am Freitag, 1. November 2013, 09:25:37 schrieb Patrick McHardy:
> On Fri, Nov 01, 2013 at 09:45:29AM +0100, Steffen Klassert wrote:
> > On Wed, Oct 30, 2013 at 12:07:11AM +0000, Patrick McHardy wrote:
> > > The problem is that the reassembled packet is referenced by the
> > > individual
> > > fragments, so we trigger the BUG_ON in pskb_expand_head(). In this
> > > particular case the case we BUG() on is actually OK, but I'm looking at
> > > a way we can fix this without special casing. Hope to have a patch for
> > > testing in the next hours.
> >
> > Just for the record. I'm observing similar, quite reproducable crashes
> > when
> > receiving fragmented icmp echo request packets on an IPsec gateway with
> > nf_conntrack_ipv6.
> >
> > Since git commit 58a317f10 ("netfilter: ipv6: add IPv6 NAT support")
> > netfilter might insert a reassembled ipv6 packet with a shared skb and
> > local_df = 1 to the ok function. In case of xfrm, __xfrm6_output()
> > fragments the packet again and when adjusting the headroom later, we
> > crash because of a shared skb.
> >
> > I can fix it by checking for a shared skb in ip6_fragment() and do
> > slow path fragmentation then. But we never needed such a check in
> > ip6_fragment(), so it's maybe better to fix it in netfilter.
>
> So what seems to be happening is that this case in __ipv6_conntrack_in()
> triggers:
>
> /* Conntrack helpers need the entire reassembled packet in the
> * POST_ROUTING hook. In case of unconfirmed connections NAT
> * might reassign a helper, so the entire packet is also
> * required.
> */
> ct = nf_ct_get(reasm, &ctinfo);
> if (ct != NULL && !nf_ct_is_untracked(ct)) {
> help = nfct_help(ct);
> if ((help && help->helper) || !nf_ct_is_confirmed(ct)) {
> nf_conntrack_get_reasm(reasm);
> NF_HOOK_THRESH(NFPROTO_IPV6, hooknum, reasm,
> (struct net_device *)in,
> (struct net_device *)out,
> okfn, NF_IP6_PRI_CONNTRACK + 1);
>
> Since this code is called while walking through the fragment chain, we have
> extra references to the reassembled skb. So I think what we need to do is
> to release the fragment chain before calling NF_HOOK_THRESH() and indicate
> this to nf_ct_frag6_output() so it will stop processing the chain
> immediately.
>
> I'll give it a try, will let you know when I have a patch for testing.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Are there patches available? I can crash my 3.12 kernel easily doing
fping -p 20 -l -b 4000 bla
3.11.x does not expose this problem.
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply
* Re: [PATCH 2/2] ipv6: use sk_v6_copy_addrs when memcpy struct ipv6_pinfo
From: Daniel Borkmann @ 2013-11-19 11:35 UTC (permalink / raw)
To: Eric Dumazet
Cc: wangweidong, davem, gerrit, kuznet, jmorris, yoshfuji, kaber,
vyasevich, nhorman, dccp, netdev, linux-sctp, dingtianhong
In-Reply-To: <1384833653.8604.84.camel@edumazet-glaptop2.roam.corp.google.com>
On 11/19/2013 05:00 AM, Eric Dumazet wrote:
> On Mon, 2013-11-18 at 19:58 -0800, Eric Dumazet wrote:
>
>> But sctp is not tcp ;)
>>
>> sctp_copy_sock() is doing some clever/partial copy of the socket, so
>> please fix it ;)
>>
>> No idea why it's not doing the normal copy of the socket.
>
> BTW, not doing the full copy means for example that SO_MAX_PACING_RATE
> is broken for SCTP.
Yep, indeed. Looking into it ...
^ permalink raw reply
* Re: [PATCH net] neigh: Force garbage collection if an entry is deleted administratively
From: Steffen Klassert @ 2013-11-19 11:54 UTC (permalink / raw)
To: David Miller; +Cc: yoshfuji, netdev
In-Reply-To: <20131118.162115.407611651189468804.davem@davemloft.net>
On Mon, Nov 18, 2013 at 04:21:15PM -0500, David Miller wrote:
>
> I think the main issue is that after this patch, the problem is really
> still there.
>
> Let's say some device holds onto the neigh for a long time, then during
> this time an administrative replacement will still get that -EEXIST
> failure.
Well, I think we had this problem even before we stopped to do
cyclic garbage collecting. This patch would just restore the
behaviour we had before this change.
>
> My conclusion is that the management of the state is the problem.
> Specifically, if we invalidate an entry then we should remove it's
> visisbility. This means the table should operate by unhashing the
> entry unconditionally during such operations.
>
> If some stray references exist, that's fine, the entity holding the
> reference will perform the final neigh cleanup at release time.
>
> Does this make sense to you?
Yes, makes sense :-)
I'm not sure how invasive this will be, so maybe we should do
one patch to restore the old behaviour (for net) and one on
top of this to improve the situation as you described
(for net-next).
^ permalink raw reply
* RE: [PATCH net] neigh: Force garbage collection if an entry is deleted administratively
From: David Laight @ 2013-11-19 12:08 UTC (permalink / raw)
To: Steffen Klassert, David Miller; +Cc: yoshfuji, netdev
In-Reply-To: <20131119115414.GZ31491@secunet.com>
> > My conclusion is that the management of the state is the problem.
> > Specifically, if we invalidate an entry then we should remove it's
> > visisbility. This means the table should operate by unhashing the
> > entry unconditionally during such operations.
> >
> > If some stray references exist, that's fine, the entity holding the
> > reference will perform the final neigh cleanup at release time.
> >
> > Does this make sense to you?
>
> Yes, makes sense :-)
Isn't it enough to act as if the entry were not in the hash tables.
So an attempt to add such an entry wouldn't fail.
I've not looked at these code paths, but it can easily be that when
the entry is invalidated the hash table isn't (and can't easily be)
locked - just having the entry locked may make it difficult.
Whereas the code path to add an entry can easily delete old entries.
David
^ permalink raw reply
* Re: ipv6 fragmentation-related panic in netfilter
From: Hannes Frederic Sowa @ 2013-11-19 12:40 UTC (permalink / raw)
To: Wolfgang Walter
Cc: Patrick McHardy, Steffen Klassert, Tomas Hlavacek, netdev,
netfilter-devel
In-Reply-To: <19807032.ebUJct12DE@h2o.as.studentenwerk.mhn.de>
On Tue, Nov 19, 2013 at 12:11:24PM +0100, Wolfgang Walter wrote:
> Are there patches available? I can crash my 3.12 kernel easily doing
>
> fping -p 20 -l -b 4000 bla
>
> 3.11.x does not expose this problem.
Yes, see here:
http://patchwork.ozlabs.org/patch/288967/
http://patchwork.ozlabs.org/patch/288970/
Greetings,
Hannes
^ permalink raw reply
* Re: [PATCH net] neigh: Force garbage collection if an entry is deleted administratively
From: Steffen Klassert @ 2013-11-19 12:41 UTC (permalink / raw)
To: David Laight; +Cc: David Miller, yoshfuji, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7432@saturn3.aculab.com>
On Tue, Nov 19, 2013 at 12:08:06PM -0000, David Laight wrote:
> > > My conclusion is that the management of the state is the problem.
> > > Specifically, if we invalidate an entry then we should remove it's
> > > visisbility. This means the table should operate by unhashing the
> > > entry unconditionally during such operations.
> > >
> > > If some stray references exist, that's fine, the entity holding the
> > > reference will perform the final neigh cleanup at release time.
> > >
> > > Does this make sense to you?
> >
> > Yes, makes sense :-)
>
> Isn't it enough to act as if the entry were not in the hash tables.
> So an attempt to add such an entry wouldn't fail.
Hm, how you want to do that?
>
> I've not looked at these code paths, but it can easily be that when
> the entry is invalidated the hash table isn't (and can't easily be)
> locked - just having the entry locked may make it difficult.
>
We have the table locked in neigh_periodic_work() so we can unlink
invalidated entries there. This function could then periodically
check and remove the unlinked entries if they lost their references.
Unlinking with neigh_periodic_work() would have some seconds delay
of course, but I think this is acceptable.
> Whereas the code path to add an entry can easily delete old entries.
>
Well, it can not if the old entry is still referenced. That's why
we need to look periodically for stale entries and remove them
when they lost their references.
^ permalink raw reply
* Re: net/usb/ax88179_178a driver broken in linux-3.12
From: Mark Lord @ 2013-11-19 13:44 UTC (permalink / raw)
To: David Laight, Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7431@saturn3.aculab.com>
[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]
On 13-11-19 05:04 AM, David Laight wrote:
>
> Which changes did you revert?
Just the bits that changed how the headroom/tailroom sizes
were checked and adjusted. See attachment for the revert patch
I am using here. My mailer unfortunately likes to mangle inline patches.
=========snip===========
Revert USB 3.0 network driver changes that break the adapter (lockups)
in 3.12. This just puts back the original code from previous kernels.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- linux/drivers/net/usb/ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:23:39.525734277 -0500
@@ -1177,18 +1177,31 @@
int frame_size = dev->maxpacket;
int mss = skb_shinfo(skb)->gso_size;
int headroom;
+ int tailroom;
tx_hdr1 = skb->len;
tx_hdr2 = mss;
if (((skb->len + 8) % frame_size) == 0)
tx_hdr2 |= 0x80008000; /* Enable padding */
- headroom = skb_headroom(skb) - 8;
+ headroom = skb_headroom(skb);
+ tailroom = skb_tailroom(skb);
- if ((skb_header_cloned(skb) || headroom < 0) &&
- pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+ if (!skb_header_cloned(skb) &&
+ !skb_cloned(skb) &&
+ (headroom + tailroom) >= 8) {
+ if (headroom < 8) {
+ skb->data = memmove(skb->head + 8, skb->data, skb->len);
+ skb_set_tail_pointer(skb, skb->len);
+ }
+ } else {
+ struct sk_buff *skb2;
+
+ skb2 = skb_copy_expand(skb, 8, 0, flags);
dev_kfree_skb_any(skb);
- return NULL;
+ skb = skb2;
+ if (!skb)
+ return NULL;
}
skb_push(skb, 4);
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
[-- Attachment #2: 51_ax88179_178a_revert_3.12_lockups.patch --]
[-- Type: text/x-patch, Size: 1250 bytes --]
Revert USB 3.0 network driver changes that break the adapter (lockups)
in 3.12. This just puts back the original code from previous kernels.
Signed-off-by: Mark Lord <mlord@pobox.com>
--- linux/drivers/net/usb/ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
+++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:23:39.525734277 -0500
@@ -1177,18 +1177,31 @@
int frame_size = dev->maxpacket;
int mss = skb_shinfo(skb)->gso_size;
int headroom;
+ int tailroom;
tx_hdr1 = skb->len;
tx_hdr2 = mss;
if (((skb->len + 8) % frame_size) == 0)
tx_hdr2 |= 0x80008000; /* Enable padding */
- headroom = skb_headroom(skb) - 8;
+ headroom = skb_headroom(skb);
+ tailroom = skb_tailroom(skb);
- if ((skb_header_cloned(skb) || headroom < 0) &&
- pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
+ if (!skb_header_cloned(skb) &&
+ !skb_cloned(skb) &&
+ (headroom + tailroom) >= 8) {
+ if (headroom < 8) {
+ skb->data = memmove(skb->head + 8, skb->data, skb->len);
+ skb_set_tail_pointer(skb, skb->len);
+ }
+ } else {
+ struct sk_buff *skb2;
+
+ skb2 = skb_copy_expand(skb, 8, 0, flags);
dev_kfree_skb_any(skb);
- return NULL;
+ skb = skb2;
+ if (!skb)
+ return NULL;
}
skb_push(skb, 4);
^ permalink raw reply
* Re: [BUG,REGRESSION?] 3.11.6+,3.12: GbE iface rate drops to few KB/s
From: Eric Dumazet @ 2013-11-19 13:53 UTC (permalink / raw)
To: Arnaud Ebalard
Cc: Willy Tarreau, Thomas Petazzoni, netdev, edumazet, Cong Wang,
linux-arm-kernel, Florian Fainelli, simon.guinot
In-Reply-To: <87li0kkhzx.fsf@natisbad.org>
On Tue, 2013-11-19 at 07:44 +0100, Arnaud Ebalard wrote:
> I did some test regarding mvneta perf on current linus tree (commit
> 2d3c627502f2a9b0, w/ c9eeec26e32e "tcp: TSQ can use a dynamic limit"
> reverted). It has Simon's tclk patch for mvebu (1022c75f5abd, "clk:
> armada-370: fix tclk frequencies"). Kernel has some debug options
> enabled and the patch above is not applied. I will spend some time on
> this two directions this evening. The idea was to get some numbers on
> the impact of TCP send window size and tcp_limit_output_bytes for
> mvneta.
Note the last patch I sent was not relevant to your problem, do not
bother trying it. Its useful for applications doing lot of consecutive
short writes, like interactive ssh launching line buffered commands.
>
>
> The test is done with a laptop (Debian, 3.11.0, e1000e) directly
> connected to a RN102 (Marvell Armada 370 @1.2GHz, mvneta). The RN102
> is running Debian armhf with an Apache2 serving a 1GB file from ext4
> over lvm over RAID1 from 2 WD30EFRX. The client is nothing fancy, i.e.
> a simple wget w/ -O /dev/null option.
>
> With the exact same setup on a ReadyNAS Duo v2 (Kirkwood 88f6282
> @1.6GHz, mv643xx_eth), I managed to get a throughput of 108MB/s
> (cannot remember the kernel version but sth between 3.8 and 3.10.
>
> So with that setup:
>
> w/ TCP send window set to 4MB: 17.4 MB/s
> w/ TCP send window set to 2MB: 16.2 MB/s
> w/ TCP send window set to 1MB: 15.6 MB/s
> w/ TCP send window set to 512KB: 25.6 MB/s
> w/ TCP send window set to 256KB: 57.7 MB/s
> w/ TCP send window set to 128KB: 54.0 MB/s
> w/ TCP send window set to 64KB: 46.2 MB/s
> w/ TCP send window set to 32KB: 42.8 MB/s
One of the problem is that tcp_sendmsg() holds the socket lock for the
whole duration of the system call if it has not to sleep. This model
doesnt allow for incoming ACKS to be processed (they are put in socket
backlog and will be processed at socket release time), and TX completion
to also queue the next chunk.
These strange results you have tend to show that if you have a big TCP
send window, the web server pushes a lot of bytes per system call and
might stall the ACK clocking or TX refills.
>
> Then, I started playing w/ tcp_limit_output_bytes (default is 131072),
> w/ TCP send window set to 256KB:
>
> tcp_limit_output_bytes set to 512KB: 59.3 MB/s
> tcp_limit_output_bytes set to 256KB: 58.5 MB/s
> tcp_limit_output_bytes set to 128KB: 56.2 MB/s
> tcp_limit_output_bytes set to 64KB: 32.1 MB/s
> tcp_limit_output_bytes set to 32KB: 4.76 MB/s
>
> As a side note, during the test, I sometimes gets peak for some seconds
> at the beginning at 90MB/s which tend to confirm what WIlly wrote,
> i.e. that the hardware can do more.
I would also check the receiver. I suspect packets drops because of a
bad driver doing skb->truesize overshooting.
nstat >/dev/null ; wget .... ; nstat
^ permalink raw reply
* RE: net/usb/ax88179_178a driver broken in linux-3.12
From: David Laight @ 2013-11-19 13:56 UTC (permalink / raw)
To: Mark Lord, Eric Dumazet, Ming Lei, davem, netdev
In-Reply-To: <528B6B5A.8000003@pobox.com>
> From: Mark Lord [mailto:mlord@pobox.com]
> On 13-11-19 05:04 AM, David Laight wrote:
> >
> > Which changes did you revert?
>
> Just the bits that changed how the headroom/tailroom sizes
> were checked and adjusted. See attachment for the revert patch
> I am using here. My mailer unfortunately likes to mangle inline patches.
>
> =========snip===========
> Revert USB 3.0 network driver changes that break the adapter (lockups)
> in 3.12. This just puts back the original code from previous kernels.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>
>
> --- linux/drivers/net/usb/ax88179_178a.c.orig 2013-11-03 18:41:51.000000000 -0500
> +++ linux/drivers/net/usb/ax88179_178a.c 2013-11-17 13:23:39.525734277 -0500
> @@ -1177,18 +1177,31 @@
> int frame_size = dev->maxpacket;
> int mss = skb_shinfo(skb)->gso_size;
> int headroom;
> + int tailroom;
>
> tx_hdr1 = skb->len;
> tx_hdr2 = mss;
> if (((skb->len + 8) % frame_size) == 0)
> tx_hdr2 |= 0x80008000; /* Enable padding */
>
> - headroom = skb_headroom(skb) - 8;
> + headroom = skb_headroom(skb);
> + tailroom = skb_tailroom(skb);
>
> - if ((skb_header_cloned(skb) || headroom < 0) &&
> - pskb_expand_head(skb, headroom < 0 ? 8 : 0, 0, GFP_ATOMIC)) {
> + if (!skb_header_cloned(skb) &&
> + !skb_cloned(skb) &&
> + (headroom + tailroom) >= 8) {
> + if (headroom < 8) {
> + skb->data = memmove(skb->head + 8, skb->data, skb->len);
> + skb_set_tail_pointer(skb, skb->len);
> + }
> + } else {
> + struct sk_buff *skb2;
> +
> + skb2 = skb_copy_expand(skb, 8, 0, flags);
> dev_kfree_skb_any(skb);
> - return NULL;
> + skb = skb2;
> + if (!skb)
> + return NULL;
> }
>
> skb_push(skb, 4);
At a guess it will be a difference between pskb_expand_head() and
skb_copy_expand(). The latter probably generates a linear skb.
The extra check for skb_cloned() probably also applies - and forces
a data copy.
Have you tried the patch to xhci-ring.c?
David
^ 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