* [PATCH 3/4] ipvs: rerouting to local clients is not needed anymore
From: Pablo Neira Ayuso @ 2015-01-31 20:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1422737711-5169-1-git-send-email-pablo@netfilter.org>
From: Julian Anastasov <ja@ssi.bg>
commit f5a41847acc5 ("ipvs: move ip_route_me_harder for ICMP")
from 2.6.37 introduced ip_route_me_harder() call for responses to
local clients, so that we can provide valid rt_src after SNAT.
It was used by TCP to provide valid daddr for ip_send_reply().
After commit 0a5ebb8000c5 ("ipv4: Pass explicit daddr arg to
ip_send_reply()." from 3.0 this rerouting is not needed anymore
and should be avoided, especially in LOCAL_IN.
Fixes 3.12.33 crash in xfrm reported by Florian Wiessner:
"3.12.33 - BUG xfrm_selector_match+0x25/0x2f6"
Reported-by: Smart Weblications GmbH - Florian Wiessner <f.wiessner@smart-weblications.de>
Tested-by: Smart Weblications GmbH - Florian Wiessner <f.wiessner@smart-weblications.de>
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Simon Horman <horms@verge.net.au>
---
net/netfilter/ipvs/ip_vs_core.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 990decb..b87ca32 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -659,16 +659,24 @@ static inline int ip_vs_gather_frags(struct sk_buff *skb, u_int32_t user)
return err;
}
-static int ip_vs_route_me_harder(int af, struct sk_buff *skb)
+static int ip_vs_route_me_harder(int af, struct sk_buff *skb,
+ unsigned int hooknum)
{
+ if (!sysctl_snat_reroute(skb))
+ return 0;
+ /* Reroute replies only to remote clients (FORWARD and LOCAL_OUT) */
+ if (NF_INET_LOCAL_IN == hooknum)
+ return 0;
#ifdef CONFIG_IP_VS_IPV6
if (af == AF_INET6) {
- if (sysctl_snat_reroute(skb) && ip6_route_me_harder(skb) != 0)
+ struct dst_entry *dst = skb_dst(skb);
+
+ if (dst->dev && !(dst->dev->flags & IFF_LOOPBACK) &&
+ ip6_route_me_harder(skb) != 0)
return 1;
} else
#endif
- if ((sysctl_snat_reroute(skb) ||
- skb_rtable(skb)->rt_flags & RTCF_LOCAL) &&
+ if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL) &&
ip_route_me_harder(skb, RTN_LOCAL) != 0)
return 1;
@@ -791,7 +799,8 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
union nf_inet_addr *snet,
__u8 protocol, struct ip_vs_conn *cp,
struct ip_vs_protocol *pp,
- unsigned int offset, unsigned int ihl)
+ unsigned int offset, unsigned int ihl,
+ unsigned int hooknum)
{
unsigned int verdict = NF_DROP;
@@ -821,7 +830,7 @@ static int handle_response_icmp(int af, struct sk_buff *skb,
#endif
ip_vs_nat_icmp(skb, pp, cp, 1);
- if (ip_vs_route_me_harder(af, skb))
+ if (ip_vs_route_me_harder(af, skb, hooknum))
goto out;
/* do the statistics and put it back */
@@ -916,7 +925,7 @@ static int ip_vs_out_icmp(struct sk_buff *skb, int *related,
snet.ip = iph->saddr;
return handle_response_icmp(AF_INET, skb, &snet, cih->protocol, cp,
- pp, ciph.len, ihl);
+ pp, ciph.len, ihl, hooknum);
}
#ifdef CONFIG_IP_VS_IPV6
@@ -981,7 +990,8 @@ static int ip_vs_out_icmp_v6(struct sk_buff *skb, int *related,
snet.in6 = ciph.saddr.in6;
writable = ciph.len;
return handle_response_icmp(AF_INET6, skb, &snet, ciph.protocol, cp,
- pp, writable, sizeof(struct ipv6hdr));
+ pp, writable, sizeof(struct ipv6hdr),
+ hooknum);
}
#endif
@@ -1040,7 +1050,8 @@ static inline bool is_new_conn(const struct sk_buff *skb,
*/
static unsigned int
handle_response(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
- struct ip_vs_conn *cp, struct ip_vs_iphdr *iph)
+ struct ip_vs_conn *cp, struct ip_vs_iphdr *iph,
+ unsigned int hooknum)
{
struct ip_vs_protocol *pp = pd->pp;
@@ -1078,7 +1089,7 @@ handle_response(int af, struct sk_buff *skb, struct ip_vs_proto_data *pd,
* if it came from this machine itself. So re-compute
* the routing information.
*/
- if (ip_vs_route_me_harder(af, skb))
+ if (ip_vs_route_me_harder(af, skb, hooknum))
goto drop;
IP_VS_DBG_PKT(10, af, pp, skb, 0, "After SNAT");
@@ -1181,7 +1192,7 @@ ip_vs_out(unsigned int hooknum, struct sk_buff *skb, int af)
cp = pp->conn_out_get(af, skb, &iph, 0);
if (likely(cp))
- return handle_response(af, skb, pd, cp, &iph);
+ return handle_response(af, skb, pd, cp, &iph, hooknum);
if (sysctl_nat_icmp_send(net) &&
(pp->protocol == IPPROTO_TCP ||
pp->protocol == IPPROTO_UDP ||
--
1.7.10.4
^ permalink raw reply related
* [PATCH 4/4] netfilter: nf_tables: fix leaks in error path of nf_tables_newchain()
From: Pablo Neira Ayuso @ 2015-01-31 20:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1422737711-5169-1-git-send-email-pablo@netfilter.org>
Release statistics and module refcount on memory allocation problems.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b543606..1ff04bc 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1264,8 +1264,10 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
nft_ctx_init(&ctx, skb, nlh, afi, table, chain, nla);
trans = nft_trans_alloc(&ctx, NFT_MSG_NEWCHAIN,
sizeof(struct nft_trans_chain));
- if (trans == NULL)
+ if (trans == NULL) {
+ free_percpu(stats);
return -ENOMEM;
+ }
nft_trans_chain_stats(trans) = stats;
nft_trans_chain_update(trans) = true;
@@ -1321,8 +1323,10 @@ static int nf_tables_newchain(struct sock *nlsk, struct sk_buff *skb,
hookfn = type->hooks[hooknum];
basechain = kzalloc(sizeof(*basechain), GFP_KERNEL);
- if (basechain == NULL)
+ if (basechain == NULL) {
+ module_put(type->owner);
return -ENOMEM;
+ }
if (nla[NFTA_CHAIN_COUNTERS]) {
stats = nft_stats_alloc(nla[NFTA_CHAIN_COUNTERS]);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/3] netlabel: Deletion of a few unnecessary checks
From: SF Markus Elfring @ 2015-01-31 21:14 UTC (permalink / raw)
To: David S. Miller, Paul Moore, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Jan 2015 22:00:14 +0100
Further update suggestions were taken into account after patches were applied
from static source code analysis.
Markus Elfring (3):
Deletion of an unnecessary check before the function call "cipso_v4_doi_putdef"
Deletion of an unnecessary check before the function call "cipso_v4_doi_free"
Less function calls in netlbl_mgmt_add_common() after error detection
net/netlabel/netlabel_cipso_v4.c | 3 +--
net/netlabel/netlabel_mgmt.c | 47 ++++++++++++++++++++--------------------
2 files changed, 24 insertions(+), 26 deletions(-)
--
2.2.2
^ permalink raw reply
* [PATCH 1/3] netlabel: Deletion of an unnecessary check before the function call "cipso_v4_doi_putdef"
From: SF Markus Elfring @ 2015-01-31 21:34 UTC (permalink / raw)
To: David S. Miller, Paul Moore, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD45B6.6010703@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Jan 2015 19:09:50 +0100
The cipso_v4_doi_putdef() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/netlabel/netlabel_mgmt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index 8b3b789..f5807f5 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -242,8 +242,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
return 0;
add_failure:
- if (cipsov4)
- cipso_v4_doi_putdef(cipsov4);
+ cipso_v4_doi_putdef(cipsov4);
if (entry)
kfree(entry->domain);
kfree(addrmap);
--
2.2.2
^ permalink raw reply related
* [PATCH 2/3] netlabel: Deletion of an unnecessary check before the function call "cipso_v4_doi_free"
From: SF Markus Elfring @ 2015-01-31 21:36 UTC (permalink / raw)
To: David S. Miller, Paul Moore, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD45B6.6010703@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Jan 2015 19:35:59 +0100
The cipso_v4_doi_free() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/netlabel/netlabel_cipso_v4.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
index 1796253..7fd1104 100644
--- a/net/netlabel/netlabel_cipso_v4.c
+++ b/net/netlabel/netlabel_cipso_v4.c
@@ -324,8 +324,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info,
return 0;
add_std_failure:
- if (doi_def)
- cipso_v4_doi_free(doi_def);
+ cipso_v4_doi_free(doi_def);
return ret_val;
}
--
2.2.2
^ permalink raw reply related
* [PATCH 3/3] netlabel: Less function calls in netlbl_mgmt_add_common() after error detection
From: SF Markus Elfring @ 2015-01-31 21:38 UTC (permalink / raw)
To: David S. Miller, Paul Moore, netdev; +Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD45B6.6010703@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 31 Jan 2015 21:55:48 +0100
The functions "cipso_v4_doi_putdef" and "kfree" could be called in some cases
by the netlbl_mgmt_add_common() function during error handling even if the
passed variables contained still a null pointer.
* This implementation detail could be improved by adjustments for jump labels.
* Let us return immediately after the first failed function call according to
the current Linux coding style convention.
* Let us delete also an unnecessary check for the variable "entry" there.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
net/netlabel/netlabel_mgmt.c | 46 ++++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
index f5807f5..17f1ed5 100644
--- a/net/netlabel/netlabel_mgmt.c
+++ b/net/netlabel/netlabel_mgmt.c
@@ -99,17 +99,15 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
u32 tmp_val;
entry = kzalloc(sizeof(*entry), GFP_KERNEL);
- if (entry == NULL) {
- ret_val = -ENOMEM;
- goto add_failure;
- }
+ if (!entry)
+ return -ENOMEM;
entry->def.type = nla_get_u32(info->attrs[NLBL_MGMT_A_PROTOCOL]);
if (info->attrs[NLBL_MGMT_A_DOMAIN]) {
size_t tmp_size = nla_len(info->attrs[NLBL_MGMT_A_DOMAIN]);
entry->domain = kmalloc(tmp_size, GFP_KERNEL);
if (entry->domain == NULL) {
ret_val = -ENOMEM;
- goto add_failure;
+ goto free_entry;
}
nla_strlcpy(entry->domain,
info->attrs[NLBL_MGMT_A_DOMAIN], tmp_size);
@@ -125,16 +123,16 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
break;
case NETLBL_NLTYPE_CIPSOV4:
if (!info->attrs[NLBL_MGMT_A_CV4DOI])
- goto add_failure;
+ goto free_domain;
tmp_val = nla_get_u32(info->attrs[NLBL_MGMT_A_CV4DOI]);
cipsov4 = cipso_v4_doi_getdef(tmp_val);
if (cipsov4 == NULL)
- goto add_failure;
+ goto free_domain;
entry->def.cipso = cipsov4;
break;
default:
- goto add_failure;
+ goto free_domain;
}
if (info->attrs[NLBL_MGMT_A_IPV4ADDR]) {
@@ -145,7 +143,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
addrmap = kzalloc(sizeof(*addrmap), GFP_KERNEL);
if (addrmap == NULL) {
ret_val = -ENOMEM;
- goto add_failure;
+ goto doi_put_def;
}
INIT_LIST_HEAD(&addrmap->list4);
INIT_LIST_HEAD(&addrmap->list6);
@@ -153,12 +151,12 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
if (nla_len(info->attrs[NLBL_MGMT_A_IPV4ADDR]) !=
sizeof(struct in_addr)) {
ret_val = -EINVAL;
- goto add_failure;
+ goto free_address_map;
}
if (nla_len(info->attrs[NLBL_MGMT_A_IPV4MASK]) !=
sizeof(struct in_addr)) {
ret_val = -EINVAL;
- goto add_failure;
+ goto free_address_map;
}
addr = nla_data(info->attrs[NLBL_MGMT_A_IPV4ADDR]);
mask = nla_data(info->attrs[NLBL_MGMT_A_IPV4MASK]);
@@ -166,7 +164,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
map = kzalloc(sizeof(*map), GFP_KERNEL);
if (map == NULL) {
ret_val = -ENOMEM;
- goto add_failure;
+ goto free_address_map;
}
map->list.addr = addr->s_addr & mask->s_addr;
map->list.mask = mask->s_addr;
@@ -178,7 +176,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = netlbl_af4list_add(&map->list, &addrmap->list4);
if (ret_val != 0) {
kfree(map);
- goto add_failure;
+ goto free_address_map;
}
entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
@@ -192,7 +190,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
addrmap = kzalloc(sizeof(*addrmap), GFP_KERNEL);
if (addrmap == NULL) {
ret_val = -ENOMEM;
- goto add_failure;
+ goto doi_put_def;
}
INIT_LIST_HEAD(&addrmap->list4);
INIT_LIST_HEAD(&addrmap->list6);
@@ -200,12 +198,12 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
if (nla_len(info->attrs[NLBL_MGMT_A_IPV6ADDR]) !=
sizeof(struct in6_addr)) {
ret_val = -EINVAL;
- goto add_failure;
+ goto free_address_map;
}
if (nla_len(info->attrs[NLBL_MGMT_A_IPV6MASK]) !=
sizeof(struct in6_addr)) {
ret_val = -EINVAL;
- goto add_failure;
+ goto free_address_map;
}
addr = nla_data(info->attrs[NLBL_MGMT_A_IPV6ADDR]);
mask = nla_data(info->attrs[NLBL_MGMT_A_IPV6MASK]);
@@ -213,7 +211,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
map = kzalloc(sizeof(*map), GFP_KERNEL);
if (map == NULL) {
ret_val = -ENOMEM;
- goto add_failure;
+ goto free_address_map;
}
map->list.addr = *addr;
map->list.addr.s6_addr32[0] &= mask->s6_addr32[0];
@@ -227,7 +225,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = netlbl_af6list_add(&map->list, &addrmap->list6);
if (ret_val != 0) {
kfree(map);
- goto add_failure;
+ goto free_address_map;
}
entry->def.type = NETLBL_NLTYPE_ADDRSELECT;
@@ -237,15 +235,17 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
ret_val = netlbl_domhsh_add(entry, audit_info);
if (ret_val != 0)
- goto add_failure;
+ goto free_address_map;
return 0;
-add_failure:
- cipso_v4_doi_putdef(cipsov4);
- if (entry)
- kfree(entry->domain);
+free_address_map:
kfree(addrmap);
+doi_put_def:
+ cipso_v4_doi_putdef(cipsov4);
+free_domain:
+ kfree(entry->domain);
+free_entry:
kfree(entry);
return ret_val;
}
--
2.2.2
^ permalink raw reply related
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: SF Markus Elfring @ 2015-01-31 21:48 UTC (permalink / raw)
To: Lino Sanfilippo, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD115C.8070801@gmx.de>
>> +exit:
>> return err;
>> }
>
> Why do you use that exit label if it does nothing more than returning
> the error value? Also if nla_parse fails you dont use it but return the
> error directly. While using a label which is used only to return an
> error may be a matter of taste, its at least inconsistent to do both in
> a function, use such a label in one case and return immediately in
> another, isnt it?
I find that all these cases correspond to the current Linux coding
style documentation, doesn't it?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: SF Markus Elfring @ 2015-01-31 21:52 UTC (permalink / raw)
To: Lino Sanfilippo, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD15EE.5040401@gmx.de>
>> -errout:
>> - if (err && meta)
>> - meta_delete(meta);
>
> Since this patch seems to be about optimization and cleanup you should
> probably also remove the now unnecessary initialization of "meta" with
> NULL at the beginning of the function...
Will the optimiser of the C compiler drop any remaining unnecessary
variable initialisations?
Do you want another update step here?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: Lino Sanfilippo @ 2015-01-31 22:09 UTC (permalink / raw)
To: SF Markus Elfring, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD4EB2.4010509@users.sourceforge.net>
On 31.01.2015 22:52, SF Markus Elfring wrote:
>>> -errout:
>>> - if (err && meta)
>>> - meta_delete(meta);
>>
>> Since this patch seems to be about optimization and cleanup you should
>> probably also remove the now unnecessary initialization of "meta" with
>> NULL at the beginning of the function...
>
> Will the optimiser of the C compiler drop any remaining unnecessary
> variable initialisations?
>
I dont know if that matters, since the code is not only used by
compilers but also read by humans.
> Do you want another update step here?
>
I am not the one who decides if this patch is acceptable or not. But I
vote for a removal of that assignment (as a part of the same patch).
Regards,
Lino
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: Lino Sanfilippo @ 2015-01-31 22:15 UTC (permalink / raw)
To: SF Markus Elfring, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD4D91.9080801@users.sourceforge.net>
On 31.01.2015 22:48, SF Markus Elfring wrote:
>
> I find that all these cases correspond to the current Linux coding
> style documentation, doesn't it?
>
Sure, I think it does. But it was not coding style violation what I was
reffering to.
Regards,
Lino
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: SF Markus Elfring @ 2015-01-31 22:20 UTC (permalink / raw)
To: Lino Sanfilippo, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD5404.1010004@gmx.de>
>> I find that all these cases correspond to the current Linux coding
>> style documentation, doesn't it?
>
> Sure, I think it does.
Thanks for your acknowledgement.
> But it was not coding style violation what I was reffering to.
Do you suggest any fine-tuning for the affected documentation
so that I would tweak my update suggestion once more?
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: Lino Sanfilippo @ 2015-01-31 22:51 UTC (permalink / raw)
To: SF Markus Elfring, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD5529.5060209@users.sourceforge.net>
On 31.01.2015 23:20, SF Markus Elfring wrote:
>>> I find that all these cases correspond to the current Linux coding
>>> style documentation, doesn't it?
>>
>> Sure, I think it does.
>
> Thanks for your acknowledgement.
>
>
>> But it was not coding style violation what I was reffering to.
>
> Do you suggest any fine-tuning for the affected documentation
> so that I would tweak my update suggestion once more?
>
No I dont think that any documentation has to be adjusted. If you agree
with me you should adjust the patch accordingly and resend it. Otherwise
keep it as it is.
Regards,
Lino
^ permalink raw reply
* Re: [PATCH] net: ipv6: Make address flushing on ifdown optional - v2
From: David Miller @ 2015-02-01 1:17 UTC (permalink / raw)
To: dsahern; +Cc: netdev, hannes
In-Reply-To: <1422504065-17445-1-git-send-email-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Wed, 28 Jan 2015 21:01:05 -0700
> @@ -3032,8 +3037,9 @@ static int addrconf_ifdown(struct net_device *dev, int how)
> {
> struct net *net = dev_net(dev);
> struct inet6_dev *idev;
> - struct inet6_ifaddr *ifa;
> + struct inet6_ifaddr *ifa, *tmp;
> int state, i;
> + struct list_head del_list;
Please order local variable declarations from longest to shortest line
(ie. reverse christmas tree) when it is easy to do so.
> + /*
> + * on NETDEV_DOWN events do not flush managed (user configured)
> + * addresses unless configured to do so. If the address is not
> + * deleted reset flags and state such that DAD is re-done on a
> + * subsequent link up.
> + */
Comments in the networking are to be formatted:
/* Like
* this.
*/
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] tcp: use SACK RTTs for CC
From: David Miller @ 2015-02-01 1:26 UTC (permalink / raw)
To: kennetkl; +Cc: netdev
In-Reply-To: <1422558483-6168-1-git-send-email-kennetkl@ifi.uio.no>
From: Kenneth Klette Jonassen <kennetkl@ifi.uio.no>
Date: Thu, 29 Jan 2015 20:08:03 +0100
> Current behavior only passes RTTs from sequentially acked data to CC.
>
> If sender gets a combined ACK for segment 1 and SACK for segment 3, then the
> computed RTT for CC is the time between sending segment 1 and receiving SACK
> for segment 3.
>
> Pass the minimum computed RTT from any acked data to CC, i.e. time between
> sending segment 3 and receiving SACK for segment 3.
>
> Signed-off-by: Kenneth Klette Jonassen <kennetkl@ifi.uio.no>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] hyperv: Fix the error processing in netvsc_send()
From: David Miller @ 2015-02-01 1:32 UTC (permalink / raw)
To: haiyangz; +Cc: olaf, netdev, jasowang, driverdev-devel, linux-kernel
In-Reply-To: <1422563689-31036-1-git-send-email-haiyangz@microsoft.com>
From: Haiyang Zhang <haiyangz@microsoft.com>
Date: Thu, 29 Jan 2015 12:34:49 -0800
> The existing code frees the skb in EAGAIN case, in which the skb will be
> retried from upper layer and used again.
> Also, the existing code doesn't free send buffer slot in error case, because
> there is no completion message for unsent packets.
> This patch fixes these problems.
>
> (Please also include this patch for stable trees. Thanks!)
>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] drivers: net: cpsw: make cpsw_ale.c a module to allow re-use on Keystone
From: David Miller @ 2015-02-01 1:36 UTC (permalink / raw)
To: m-karicheri2
Cc: w-kwok2, mugunthanvnm, tony, prabhakar.csengg, grygorii.strashko,
lokeshvutla, mpa, lsorense, netdev, linux-kernel, arnd
In-Reply-To: <1422573351-5603-2-git-send-email-m-karicheri2@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Thu, 29 Jan 2015 18:15:51 -0500
> NetCP on Keystone has cpsw ale function similar to other TI SoCs
> and this driver is re-used. To allow both ti cpsw and keystone netcp
> to re-use the driver, convert the cpsw ale to a module and configure
> it through Kconfig option CONFIG_TI_CPSW_ALE. Currently it is statically
> linked to both TI CPSW and NetCP and this causes issues when the above
> drivers are built as dynamic modules. This patch addresses this issue
>
> While at it, fix the Makefile and code to build both netcp_core and
> netcp_ethss as dynamic modules. This is needed to support arm allmodconfig.
> This also requires exporting of API calls provided by netcp_core so that
> both the above can be dynamic modules.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Applied, thanks
^ permalink raw reply
* Re: [PATCH net-next v2 0/6] net: Add STT support.
From: David Miller @ 2015-02-01 1:40 UTC (permalink / raw)
To: pshelar; +Cc: netdev
In-Reply-To: <1422574156-1831-1-git-send-email-pshelar@nicira.com>
From: Pravin B Shelar <pshelar@nicira.com>
Date: Thu, 29 Jan 2015 15:29:16 -0800
> Following patch series adds support for Stateless Transport
> Tunneling protocol.
> STT uses TCP segmentation offload available in most of NIC. On
> packet xmit STT driver appends STT header along with TCP header
> to the packet. For GSO packet GSO parameters are set according
> to tunnel configuration and packet is handed over to networking
> stack. This allows use of segmentation offload available in NICs
I don't like this at all.
Routers _absolutely_ depend upon the ability to make TCP flows back
off by dropping packets in various ways (tail drop, RED random drops,
etc.). STT violates this completely.
It's _NOT_ TCP, you can't put lipstick on a pig and pretend it's not a
pig. You need to use something that indicates it's flow properties,
a datagram protocol. Either via an existing one or by creating a new
one.
I know you want to do this mass TCP behavioral violation because of
TSO. But that's too bad. The ends do not justify the means.
I also don't buy the argument that "people can put arbitrary changes
into their kernel to do stuff like that".
They can't do it to the stack I'm willing to maintain, and that's what
matters for %99 of systems out there.
^ permalink raw reply
* Re: [PATCH net-next] ipv4: icmp: use percpu allocation
From: David Miller @ 2015-02-01 1:48 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1422575889.21689.44.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 29 Jan 2015 15:58:09 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Get rid of nr_cpu_ids and use modern percpu allocation.
>
> Note that the sockets themselves are not yet allocated
> using NUMA affinity.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net: sched: fix panic in rate estimators
From: David Miller @ 2015-02-01 1:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev, john.fastabend
In-Reply-To: <1422581412.21689.53.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 29 Jan 2015 17:30:12 -0800
> From: Eric Dumazet <edumazet@google.com>
>
> Doing the following commands on a non idle network device
> panics the box instantly, because cpu_bstats gets overwritten
> by stats.
>
> tc qdisc add dev eth0 root <your_favorite_qdisc>
> ... some traffic (one packet is enough) ...
> tc qdisc replace dev eth0 root est 1sec 4sec <your_favorite_qdisc>
...
> Lets play safe and not use an union : percpu 'pointers' are mostly read
> anyway, and we have typically few qdiscs per host.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: John Fastabend <john.fastabend@gmail.com>
> Fixes: 22e0f8b9322c ("net: sched: make bstats per cpu and estimator RCU safe")
Applied and queued up for -stable, thanks Eric.
^ permalink raw reply
* Re: [PATCH 1/3] netlabel: Deletion of an unnecessary check before the function call "cipso_v4_doi_putdef"
From: Paul Moore @ 2015-02-01 2:20 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD4A7D.8000802@users.sourceforge.net>
On Sat, Jan 31, 2015 at 4:34 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 19:09:50 +0100
>
> The cipso_v4_doi_putdef() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/netlabel/netlabel_mgmt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index 8b3b789..f5807f5 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
> @@ -242,8 +242,7 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
> return 0;
>
> add_failure:
> - if (cipsov4)
> - cipso_v4_doi_putdef(cipsov4);
> + cipso_v4_doi_putdef(cipsov4);
> if (entry)
> kfree(entry->domain);
> kfree(addrmap);
> --
> 2.2.2
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 2/3] netlabel: Deletion of an unnecessary check before the function call "cipso_v4_doi_free"
From: Paul Moore @ 2015-02-01 2:22 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD4AF0.7070102@users.sourceforge.net>
On Sat, Jan 31, 2015 at 4:36 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 19:35:59 +0100
>
> The cipso_v4_doi_free() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/netlabel/netlabel_cipso_v4.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_cipso_v4.c b/net/netlabel/netlabel_cipso_v4.c
> index 1796253..7fd1104 100644
> --- a/net/netlabel/netlabel_cipso_v4.c
> +++ b/net/netlabel/netlabel_cipso_v4.c
> @@ -324,8 +324,7 @@ static int netlbl_cipsov4_add_std(struct genl_info *info,
> return 0;
>
> add_std_failure:
> - if (doi_def)
> - cipso_v4_doi_free(doi_def);
> + cipso_v4_doi_free(doi_def);
> return ret_val;
> }
>
> --
> 2.2.2
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 3/3] netlabel: Less function calls in netlbl_mgmt_add_common() after error detection
From: Paul Moore @ 2015-02-01 2:40 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, netdev, LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD4B5F.9010502@users.sourceforge.net>
On Sat, Jan 31, 2015 at 4:38 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 21:55:48 +0100
>
> The functions "cipso_v4_doi_putdef" and "kfree" could be called in some cases
> by the netlbl_mgmt_add_common() function during error handling even if the
> passed variables contained still a null pointer.
>
> * This implementation detail could be improved by adjustments for jump labels.
>
> * Let us return immediately after the first failed function call according to
> the current Linux coding style convention.
>
> * Let us delete also an unnecessary check for the variable "entry" there.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/netlabel/netlabel_mgmt.c | 46 ++++++++++++++++++++++----------------------
> 1 file changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/net/netlabel/netlabel_mgmt.c b/net/netlabel/netlabel_mgmt.c
> index f5807f5..17f1ed5 100644
> --- a/net/netlabel/netlabel_mgmt.c
> +++ b/net/netlabel/netlabel_mgmt.c
...
> @@ -237,15 +235,17 @@ static int netlbl_mgmt_add_common(struct genl_info *info,
>
> ret_val = netlbl_domhsh_add(entry, audit_info);
> if (ret_val != 0)
> - goto add_failure;
> + goto free_address_map;
>
> return 0;
>
> -add_failure:
> - cipso_v4_doi_putdef(cipsov4);
> - if (entry)
> - kfree(entry->domain);
> +free_address_map:
> kfree(addrmap);
> +doi_put_def:
> + cipso_v4_doi_putdef(cipsov4);
> +free_domain:
> + kfree(entry->domain);
> +free_entry:
> kfree(entry);
> return ret_val;
> }
It would be nice if you could stick with the goto label naming style
in the rest of the file, e.g.
"add_free_addrmap"/"add_put_doi_def"/"add_free_domain"/"add_free_entry".
--
paul moore
www.paul-moore.com
^ permalink raw reply
* AF_NETDEV - device specific sockets
From: Zayats, Michael @ 2015-02-01 4:20 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hi,
I am looking for a generic mechanism that would allow network device drivers to provide socket interface to user and kernel space clients.
Such an interface might be used to provide access to important sub-streams of packets, alongside with device specific packet metadata, provided through msg_control fields of recv/sendmsg.
RX Metadata might include device specific information, such as queuing priorities applied, potential destination interface in case of switching hardware etc.
On the transmission, metadata might be used to indicate hardware specific required optimizations, as well as any other transformation or accounting required on the packet.
AF_PACKET based mechanism doesn't allow metadata to be exchanged between the client and the device driver.
Extending it would require extending of sk_buff and potentially additional per packet operations.
Generic Netlink is not intended to pass packets.
As I am trying to validate generic applicability of such a mechanism, I see that TUN driver is providing custom socket interface, in order to deal with user information through msg_control.
Only usable inside the kernel, through custom interface.
Proposed interface
------------------
Kernel side:
(struct proto *) should be added to struct net_device.
Device driver that is interested to support socket interface would populate the pointer.
User space:
After creating AF_NETDEV socket, the only successful operation would be setting SO_BINDTODEVICE option.
Once set, all socket operations would be implemented by calling functions, that are registered at struct proto on the appropriate net_device.
What do you think?
Would you see a better approach?
Some other mechanism that already exists for such a purpose?
Thanks,
Michael
^ permalink raw reply
* Re: AF_NETDEV - device specific sockets
From: John Fastabend @ 2015-02-01 4:41 UTC (permalink / raw)
To: Zayats, Michael; +Cc: netdev@vger.kernel.org
In-Reply-To: <FC8E8D0ECC753F45808079B18C3203FE1CCA0C@G4W3293.americas.hpqcorp.net>
On 01/31/2015 08:20 PM, Zayats, Michael wrote:
> Hi,
>
> I am looking for a generic mechanism that would allow network device
> drivers to provide socket interface to user and kernel space
> clients.
>
> Such an interface might be used to provide access to important
> sub-streams of packets, alongside with device specific packet
> metadata, provided through msg_control fields of recv/sendmsg.
>
> RX Metadata might include device specific information, such as
> queuing priorities applied, potential destination interface in case
> of switching hardware etc.
>
> On the transmission, metadata might be used to indicate hardware
> specific required optimizations, as well as any other transformation
> or accounting required on the packet.
>
> AF_PACKET based mechanism doesn't allow metadata to be exchanged
> between the client and the device driver. Extending it would require
> extending of sk_buff and potentially additional per packet
> operations. Generic Netlink is not intended to pass packets.
>
> As I am trying to validate generic applicability of such a mechanism,
> I see that TUN driver is providing custom socket interface, in order
> to deal with user information through msg_control. Only usable inside
> the kernel, through custom interface.
> Proposed interface
> ------------------
> Kernel side:
> (struct proto *) should be added to struct net_device.
> Device driver that is interested to support socket interface would populate the pointer.
>
> User space: After creating AF_NETDEV socket, the only successful
> operation would be setting SO_BINDTODEVICE option. Once set, all
> socket operations would be implemented by calling functions, that are
> registered at struct proto on the appropriate net_device.
>
> What do you think?
> Would you see a better approach?
> Some other mechanism that already exists for such a purpose?
It might help to come up with specific examples but an alternate
proposal would be to use skb->priority field and then mqprio to
steer the traffic to a specific queue and then bind attributes to
the queue.
For example the NIC offloaded QOS can be mapped on to queues and
then sockets mapped to the queues.
Another example would be to forward all traffic from one queue
to a virtual fuction in SR-IOV use case. We don't have an interface
to do this but I have been working on an API that could be used
for this.
In this case you don't need to modify AF_PACKET interface but
configure the device correctly. If you need per-packet control
you could use 'tc' or 'nftables' to do the steering.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH net] net: sched: fix panic in rate estimators
From: John Fastabend @ 2015-02-01 4:45 UTC (permalink / raw)
To: eric.dumazet; +Cc: David Miller, netdev
In-Reply-To: <20150131.175520.1521332119325954392.davem@davemloft.net>
On 01/31/2015 05:55 PM, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Thu, 29 Jan 2015 17:30:12 -0800
>
>> From: Eric Dumazet <edumazet@google.com>
>>
>> Doing the following commands on a non idle network device
>> panics the box instantly, because cpu_bstats gets overwritten
>> by stats.
>>
>> tc qdisc add dev eth0 root <your_favorite_qdisc>
>> ... some traffic (one packet is enough) ...
>> tc qdisc replace dev eth0 root est 1sec 4sec <your_favorite_qdisc>
> ...
>> Lets play safe and not use an union : percpu 'pointers' are mostly read
>> anyway, and we have typically few qdiscs per host.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
>> Cc: John Fastabend <john.fastabend@gmail.com>
>> Fixes: 22e0f8b9322c ("net: sched: make bstats per cpu and estimator RCU safe")
>
> Applied and queued up for -stable, thanks Eric.
>
Thanks Eric, looks like my "replace" test scripts never sent traffic
before the replace call.
Also I haven't seen any RCU splats in awhile so I'll send out a patch
to drop the qdisc_lock on ing_filter shortly.
--
John Fastabend Intel Corporation
^ 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