* 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: 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: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: 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: 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: 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
* [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
* [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 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 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 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 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 1/4] netfilter: nf_tables: validate hooks in NAT expressions
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>
The user can crash the kernel if it uses any of the existing NAT
expressions from the wrong hook, so add some code to validate this
when loading the rule.
This patch introduces nft_chain_validate_hooks() which is based on
an existing function in the bridge version of the reject expression.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
include/net/netfilter/nf_tables.h | 2 ++
net/bridge/netfilter/nft_reject_bridge.c | 29 +++++-----------------
net/netfilter/nf_tables_api.c | 18 ++++++++++++++
net/netfilter/nft_masq.c | 26 ++++++++++++-------
net/netfilter/nft_nat.c | 40 ++++++++++++++++++++++--------
net/netfilter/nft_redir.c | 25 +++++++++++++------
6 files changed, 90 insertions(+), 50 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 3ae969e..9eaaa78 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -530,6 +530,8 @@ enum nft_chain_type {
int nft_chain_validate_dependency(const struct nft_chain *chain,
enum nft_chain_type type);
+int nft_chain_validate_hooks(const struct nft_chain *chain,
+ unsigned int hook_flags);
struct nft_stats {
u64 bytes;
diff --git a/net/bridge/netfilter/nft_reject_bridge.c b/net/bridge/netfilter/nft_reject_bridge.c
index b0330ae..3244aea 100644
--- a/net/bridge/netfilter/nft_reject_bridge.c
+++ b/net/bridge/netfilter/nft_reject_bridge.c
@@ -265,22 +265,12 @@ out:
data[NFT_REG_VERDICT].verdict = NF_DROP;
}
-static int nft_reject_bridge_validate_hooks(const struct nft_chain *chain)
+static int nft_reject_bridge_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data)
{
- struct nft_base_chain *basechain;
-
- if (chain->flags & NFT_BASE_CHAIN) {
- basechain = nft_base_chain(chain);
-
- switch (basechain->ops[0].hooknum) {
- case NF_BR_PRE_ROUTING:
- case NF_BR_LOCAL_IN:
- break;
- default:
- return -EOPNOTSUPP;
- }
- }
- return 0;
+ return nft_chain_validate_hooks(ctx->chain, (1 << NF_BR_PRE_ROUTING) |
+ (1 << NF_BR_LOCAL_IN));
}
static int nft_reject_bridge_init(const struct nft_ctx *ctx,
@@ -290,7 +280,7 @@ static int nft_reject_bridge_init(const struct nft_ctx *ctx,
struct nft_reject *priv = nft_expr_priv(expr);
int icmp_code, err;
- err = nft_reject_bridge_validate_hooks(ctx->chain);
+ err = nft_reject_bridge_validate(ctx, expr, NULL);
if (err < 0)
return err;
@@ -341,13 +331,6 @@ nla_put_failure:
return -1;
}
-static int nft_reject_bridge_validate(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nft_data **data)
-{
- return nft_reject_bridge_validate_hooks(ctx->chain);
-}
-
static struct nft_expr_type nft_reject_bridge_type;
static const struct nft_expr_ops nft_reject_bridge_ops = {
.type = &nft_reject_bridge_type,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 3b3ddb4..7e68694 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3753,6 +3753,24 @@ int nft_chain_validate_dependency(const struct nft_chain *chain,
}
EXPORT_SYMBOL_GPL(nft_chain_validate_dependency);
+int nft_chain_validate_hooks(const struct nft_chain *chain,
+ unsigned int hook_flags)
+{
+ struct nft_base_chain *basechain;
+
+ if (chain->flags & NFT_BASE_CHAIN) {
+ basechain = nft_base_chain(chain);
+
+ if ((1 << basechain->ops[0].hooknum) & hook_flags)
+ return 0;
+
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nft_chain_validate_hooks);
+
/*
* Loop detection - walk through the ruleset beginning at the destination chain
* of a new jump until either the source chain is reached (loop) or all
diff --git a/net/netfilter/nft_masq.c b/net/netfilter/nft_masq.c
index d1ffd5e..9aea747 100644
--- a/net/netfilter/nft_masq.c
+++ b/net/netfilter/nft_masq.c
@@ -21,6 +21,21 @@ const struct nla_policy nft_masq_policy[NFTA_MASQ_MAX + 1] = {
};
EXPORT_SYMBOL_GPL(nft_masq_policy);
+int nft_masq_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data)
+{
+ int err;
+
+ err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
+ if (err < 0)
+ return err;
+
+ return nft_chain_validate_hooks(ctx->chain,
+ (1 << NF_INET_POST_ROUTING));
+}
+EXPORT_SYMBOL_GPL(nft_masq_validate);
+
int nft_masq_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
@@ -28,8 +43,8 @@ int nft_masq_init(const struct nft_ctx *ctx,
struct nft_masq *priv = nft_expr_priv(expr);
int err;
- err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
- if (err < 0)
+ err = nft_masq_validate(ctx, expr, NULL);
+ if (err)
return err;
if (tb[NFTA_MASQ_FLAGS] == NULL)
@@ -60,12 +75,5 @@ nla_put_failure:
}
EXPORT_SYMBOL_GPL(nft_masq_dump);
-int nft_masq_validate(const struct nft_ctx *ctx, const struct nft_expr *expr,
- const struct nft_data **data)
-{
- return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
-}
-EXPORT_SYMBOL_GPL(nft_masq_validate);
-
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>");
diff --git a/net/netfilter/nft_nat.c b/net/netfilter/nft_nat.c
index aff54fb1..a0837c6 100644
--- a/net/netfilter/nft_nat.c
+++ b/net/netfilter/nft_nat.c
@@ -88,17 +88,40 @@ static const struct nla_policy nft_nat_policy[NFTA_NAT_MAX + 1] = {
[NFTA_NAT_FLAGS] = { .type = NLA_U32 },
};
-static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
- const struct nlattr * const tb[])
+static int nft_nat_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data)
{
struct nft_nat *priv = nft_expr_priv(expr);
- u32 family;
int err;
err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
if (err < 0)
return err;
+ switch (priv->type) {
+ case NFT_NAT_SNAT:
+ err = nft_chain_validate_hooks(ctx->chain,
+ (1 << NF_INET_POST_ROUTING) |
+ (1 << NF_INET_LOCAL_IN));
+ break;
+ case NFT_NAT_DNAT:
+ err = nft_chain_validate_hooks(ctx->chain,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_OUT));
+ break;
+ }
+
+ return err;
+}
+
+static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
+ const struct nlattr * const tb[])
+{
+ struct nft_nat *priv = nft_expr_priv(expr);
+ u32 family;
+ int err;
+
if (tb[NFTA_NAT_TYPE] == NULL ||
(tb[NFTA_NAT_REG_ADDR_MIN] == NULL &&
tb[NFTA_NAT_REG_PROTO_MIN] == NULL))
@@ -115,6 +138,10 @@ static int nft_nat_init(const struct nft_ctx *ctx, const struct nft_expr *expr,
return -EINVAL;
}
+ err = nft_nat_validate(ctx, expr, NULL);
+ if (err < 0)
+ return err;
+
if (tb[NFTA_NAT_FAMILY] == NULL)
return -EINVAL;
@@ -219,13 +246,6 @@ nla_put_failure:
return -1;
}
-static int nft_nat_validate(const struct nft_ctx *ctx,
- const struct nft_expr *expr,
- const struct nft_data **data)
-{
- return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
-}
-
static struct nft_expr_type nft_nat_type;
static const struct nft_expr_ops nft_nat_ops = {
.type = &nft_nat_type,
diff --git a/net/netfilter/nft_redir.c b/net/netfilter/nft_redir.c
index 9e8093f..d7e9e93 100644
--- a/net/netfilter/nft_redir.c
+++ b/net/netfilter/nft_redir.c
@@ -23,6 +23,22 @@ const struct nla_policy nft_redir_policy[NFTA_REDIR_MAX + 1] = {
};
EXPORT_SYMBOL_GPL(nft_redir_policy);
+int nft_redir_validate(const struct nft_ctx *ctx,
+ const struct nft_expr *expr,
+ const struct nft_data **data)
+{
+ int err;
+
+ err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
+ if (err < 0)
+ return err;
+
+ return nft_chain_validate_hooks(ctx->chain,
+ (1 << NF_INET_PRE_ROUTING) |
+ (1 << NF_INET_LOCAL_OUT));
+}
+EXPORT_SYMBOL_GPL(nft_redir_validate);
+
int nft_redir_init(const struct nft_ctx *ctx,
const struct nft_expr *expr,
const struct nlattr * const tb[])
@@ -30,7 +46,7 @@ int nft_redir_init(const struct nft_ctx *ctx,
struct nft_redir *priv = nft_expr_priv(expr);
int err;
- err = nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
+ err = nft_redir_validate(ctx, expr, NULL);
if (err < 0)
return err;
@@ -88,12 +104,5 @@ nla_put_failure:
}
EXPORT_SYMBOL_GPL(nft_redir_dump);
-int nft_redir_validate(const struct nft_ctx *ctx, const struct nft_expr *expr,
- const struct nft_data **data)
-{
- return nft_chain_validate_dependency(ctx->chain, NFT_CHAIN_T_NAT);
-}
-EXPORT_SYMBOL_GPL(nft_redir_validate);
-
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com>");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/4] Netfilter/IPVS fixes for net
From: Pablo Neira Ayuso @ 2015-01-31 20:55 UTC (permalink / raw)
To: netfilter-devel; +Cc: davem, netdev
Hi David,
The following patchset contains Netfilter/IPVS fixes for your net tree,
they are:
1) Validate hooks for nf_tables NAT expressions, otherwise users can
crash the kernel when using them from the wrong hook. We already
got one user trapped on this when configuring masquerading.
2) Fix a BUG splat in nf_tables with CONFIG_DEBUG_PREEMPT=y. Reported
by Andreas Schultz.
3) Avoid unnecessary reroute of traffic in the local input path
in IPVS that triggers a crash in in xfrm. Reported by Florian
Wiessner and fixes by Julian Anastasov.
4) Fix memory and module refcount leak from the error path of
nf_tables_newchain().
You can pull these changes from:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Thanks!
----------------------------------------------------------------
The following changes since commit 2061dcd6bff8b774b4fac8b0739b6be3f87bc9f2:
net: sctp: fix race for one-to-many sockets in sendmsg's auto associate (2015-01-17 23:52:20 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master
for you to fetch changes up to f5553c19ff9058136e7082c0b1f4268e705ea538:
netfilter: nf_tables: fix leaks in error path of nf_tables_newchain() (2015-01-30 18:42:08 +0100)
----------------------------------------------------------------
Julian Anastasov (1):
ipvs: rerouting to local clients is not needed anymore
Pablo Neira Ayuso (3):
netfilter: nf_tables: validate hooks in NAT expressions
netfilter: nf_tables: disable preemption when restoring chain counters
netfilter: nf_tables: fix leaks in error path of nf_tables_newchain()
include/net/netfilter/nf_tables.h | 2 ++
net/bridge/netfilter/nft_reject_bridge.c | 29 +++++-----------------
net/netfilter/ipvs/ip_vs_core.c | 33 ++++++++++++++++--------
net/netfilter/nf_tables_api.c | 28 +++++++++++++++++++--
net/netfilter/nft_masq.c | 26 ++++++++++++-------
net/netfilter/nft_nat.c | 40 ++++++++++++++++++++++--------
net/netfilter/nft_redir.c | 25 +++++++++++++------
7 files changed, 120 insertions(+), 63 deletions(-)
^ permalink raw reply
* [PATCH 2/4] netfilter: nf_tables: disable preemption when restoring chain counters
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>
With CONFIG_DEBUG_PREEMPT=y
[22144.496057] BUG: using smp_processor_id() in preemptible [00000000] code: iptables-compat/10406
[22144.496061] caller is debug_smp_processor_id+0x17/0x1b
[22144.496065] CPU: 2 PID: 10406 Comm: iptables-compat Not tainted 3.19.0-rc4+ #
[...]
[22144.496092] Call Trace:
[22144.496098] [<ffffffff8145b9fa>] dump_stack+0x4f/0x7b
[22144.496104] [<ffffffff81244f52>] check_preemption_disabled+0xd6/0xe8
[22144.496110] [<ffffffff81244f90>] debug_smp_processor_id+0x17/0x1b
[22144.496120] [<ffffffffa07c557e>] nft_stats_alloc+0x94/0xc7 [nf_tables]
[22144.496130] [<ffffffffa07c73d2>] nf_tables_newchain+0x471/0x6d8 [nf_tables]
[22144.496140] [<ffffffffa07c5ef6>] ? nft_trans_alloc+0x18/0x34 [nf_tables]
[22144.496154] [<ffffffffa063c8da>] nfnetlink_rcv_batch+0x2b4/0x457 [nfnetlink]
Reported-by: Andreas Schultz <aschultz@tpip.net>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
net/netfilter/nf_tables_api.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 7e68694..b543606 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1134,9 +1134,11 @@ static struct nft_stats __percpu *nft_stats_alloc(const struct nlattr *attr)
/* Restore old counters on this cpu, no problem. Per-cpu statistics
* are not exposed to userspace.
*/
+ preempt_disable();
stats = this_cpu_ptr(newstats);
stats->bytes = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_BYTES]));
stats->pkts = be64_to_cpu(nla_get_be64(tb[NFTA_COUNTER_PACKETS]));
+ preempt_enable();
return newstats;
}
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCH net-next 5/6] udpv6: Add lockless sendmsg() support
From: Sergei Shtylyov @ 2015-01-31 20:49 UTC (permalink / raw)
To: Vladislav Yasevich, netdev; +Cc: herbert, hannes, Vladislav Yasevich
In-Reply-To: <1422718818-21093-6-git-send-email-vyasevic@redhat.com>
Hello.
On 1/31/2015 6:40 PM, Vladislav Yasevich wrote:
> This commit adds the same functionaliy to IPv6 that
> commit 903ab86d195cca295379699299c5fc10beba31c7
> Author: Herbert Xu <herbert@gondor.apana.org.au>
> Date: Tue Mar 1 02:36:48 2011 +0000
> udp: Add lockless transmit path
> added to IPv4.
> UDP transmit path can now run without a socket lock,
> thus allowing multiple threads to send to a single socket
> more efficiently.
> This is only used when corking/MSG_MORE is not used.
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> net/ipv6/udp.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 67a3d70..d048d46 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
[...]
> @@ -1307,6 +1308,20 @@ do_udp_sendmsg:
> goto do_confirm;
> back_from_confirm:
>
> + /* Lockless fast path for the non-corking case */
> + if (!corkreq) {
> + struct sk_buff *skb;
> +
> + skb = ip6_make_skb(sk, getfrag, msg, ulen,
> + sizeof(struct udphdr), hlimit, tclass, opt,
> + &fl6, (struct rt6_info *)dst,
> + msg->msg_flags, dontfrag);
> + err = PTR_ERR(skb);
You should use PTR_ERR_OR_ZERO() here, I think.
[...]
WBR, Sergei
^ permalink raw reply
* Re: [GIT PULL nf] IPVS Fixes for v3.19
From: Pablo Neira Ayuso @ 2015-01-31 20:40 UTC (permalink / raw)
To: Simon Horman
Cc: lvs-devel, netdev, netfilter-devel, Wensong Zhang,
Julian Anastasov
In-Reply-To: <1422580938-31520-1-git-send-email-horms@verge.net.au>
On Fri, Jan 30, 2015 at 10:22:17AM +0900, Simon Horman wrote:
> Hi Pablo,
>
> please consider this fix for v3.19.
>
> It resolves a crash in xfrm reported by Florian Wiessner.
>
> I believe this problem manifests since 0a5ebb8000c5 ("ipv4: Pass explicit
> daddr arg to ip_send_reply().") which was included in v2.6.39.
>
> Julian reports that the patch has been tested on net tree (Dec 7), 3.14.25,
> 3.12.33, 3.10.61. 3.4.104 needs a modified fix that resolves rejects. It
> applies with little fuzz on 3.2.64. Please let us know if we should post
> separate 3.2 and 3.4 patches.
Pulled, thanks Simon.
^ permalink raw reply
* Re: [PATCH] net: sctp: Deletion of an unnecessary check before the function call "kfree"
From: Neil Horman @ 2015-01-31 18:38 UTC (permalink / raw)
To: SF Markus Elfring
Cc: David S. Miller, Vlad Yasevich, linux-sctp, netdev, LKML,
kernel-janitors, Julia Lawall
In-Reply-To: <54CD0DCF.9020804@users.sourceforge.net>
On Sat, Jan 31, 2015 at 06:15:59PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 18:10:03 +0100
>
> The kfree() 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/sctp/associola.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index f791edd..3c2aefc 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -391,8 +391,7 @@ void sctp_association_free(struct sctp_association *asoc)
> sctp_asconf_queue_teardown(asoc);
>
> /* Free pending address space being deleted */
> - if (asoc->asconf_addr_del_pending != NULL)
> - kfree(asoc->asconf_addr_del_pending);
> + kfree(asoc->asconf_addr_del_pending);
>
> /* AUTH - Free the endpoint shared keys */
> sctp_auth_destroy_keys(&asoc->endpoint_shared_keys);
> --
> 2.2.2
>
>
Sure, seems reasonable
Acked-By: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] net: sched: One function call less in em_meta_change() after error detection
From: Lino Sanfilippo @ 2015-01-31 17:50 UTC (permalink / raw)
To: SF Markus Elfring, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD042E.6030606@users.sourceforge.net>
On 31.01.2015 17:34, 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...
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 17:31 UTC (permalink / raw)
To: SF Markus Elfring, David S. Miller, Jamal Hadi Salim, netdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <54CD042E.6030606@users.sourceforge.net>
Hi,
On 31.01.2015 17:34, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 31 Jan 2015 17:18:48 +0100
>
> The meta_delete() function could be called in four cases by the
> em_meta_change() function during error handling even if the passed
> variable "meta" 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 unnecessary checks for the variables "err" and
> "meta" there.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> net/sched/em_meta.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/net/sched/em_meta.c b/net/sched/em_meta.c
> index b5294ce..2aa67b1 100644
> --- a/net/sched/em_meta.c
> +++ b/net/sched/em_meta.c
> @@ -866,23 +866,23 @@ static int em_meta_change(struct net *net, void *data, int len,
>
> err = nla_parse(tb, TCA_EM_META_MAX, data, len, meta_policy);
> if (err < 0)
> - goto errout;
> + return err;
>
> err = -EINVAL;
> if (tb[TCA_EM_META_HDR] == NULL)
> - goto errout;
> + goto exit;
> hdr = nla_data(tb[TCA_EM_META_HDR]);
>
> if (TCF_META_TYPE(hdr->left.kind) != TCF_META_TYPE(hdr->right.kind) ||
> TCF_META_TYPE(hdr->left.kind) > TCF_META_TYPE_MAX ||
> TCF_META_ID(hdr->left.kind) > TCF_META_ID_MAX ||
> TCF_META_ID(hdr->right.kind) > TCF_META_ID_MAX)
> - goto errout;
> + goto exit;
>
> meta = kzalloc(sizeof(*meta), GFP_KERNEL);
> if (meta == NULL) {
> err = -ENOMEM;
> - goto errout;
> + goto exit;
> }
>
> memcpy(&meta->lvalue.hdr, &hdr->left, sizeof(hdr->left));
> @@ -891,20 +891,20 @@ static int em_meta_change(struct net *net, void *data, int len,
> if (!meta_is_supported(&meta->lvalue) ||
> !meta_is_supported(&meta->rvalue)) {
> err = -EOPNOTSUPP;
> - goto errout;
> + goto delete_meta;
> }
>
> if (meta_change_data(&meta->lvalue, tb[TCA_EM_META_LVALUE]) < 0 ||
> meta_change_data(&meta->rvalue, tb[TCA_EM_META_RVALUE]) < 0)
> - goto errout;
> + goto delete_meta;
>
> m->datalen = sizeof(*meta);
> m->data = (unsigned long) meta;
>
> - err = 0;
> -errout:
> - if (err && meta)
> - meta_delete(meta);
> + return 0;
> +delete_meta:
> + meta_delete(meta);
> +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?
Regards,
Lino
^ permalink raw reply
* Re: [PATCH v2 0/3] Restore UFO support to virtio_net devices
From: Michael S. Tsirkin @ 2015-01-31 17:17 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: eric.dumazet, netdev, virtualization, hannes, ben, David Miller
In-Reply-To: <1422646047-13168-1-git-send-email-vyasevic@redhat.com>
On Fri, Jan 30, 2015 at 02:27:24PM -0500, Vladislav Yasevich wrote:
> commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4
> Author: Ben Hutchings <ben@decadent.org.uk>
> Date: Thu Oct 30 18:27:12 2014 +0000
>
> drivers/net: Disable UFO through virtio
>
> Turned off UFO support to virtio-net based devices due to issues
> with IPv6 fragment id generation for UFO packets. The issue
> was that IPv6 UFO/GSO implementation expects the fragment id
> to be supplied in skb_shinfo(). However, for packets generated
> by the VMs, the fragment id is not supplied which causes all
> IPv6 fragments to have the id of 0.
>
> The problem is that turning off UFO support on tap/macvtap
> as well as virtio devices caused issues with migrations.
> Migrations would fail when moving a vm from a kernel supporting
> expecting UFO to work to the newer kernels that disabled UFO.
>
> This series provides a partial solution to address the migration
> issue. The series allows us to track whether skb_shinfo()->ip6_frag_id
> has been set by treating value of 0 as unset.
> This lets GSO code to generate fragment ids if they are necessary
> (ex: packet was generated by VM or packet socket).
>
> Since v1:
> - Removed the skb bit and use value of 0 as tracker.
> - Used Eric's suggestion to set fragment id as 0x80000000 if id
> generation procedure yeilded a 0 result.
> - Consolidated ipv6 id genration code.
Looks good to me
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Seems appropriate for stable as well.
> Vladislav Yasevich (3):
> ipv6: Select fragment id during UFO segmentation if not set.
> Revert "drivers/net, ipv6: Select IPv6 fragment idents for virtio UFO
> packets"
> Revert "drivers/net: Disable UFO through virtio"
>
> drivers/net/macvtap.c | 16 ++++++++--------
> drivers/net/tun.c | 25 +++++++++----------------
> drivers/net/virtio_net.c | 24 ++++++++++--------------
> include/net/ipv6.h | 3 +++
> net/ipv6/ip6_output.c | 10 ++++------
> net/ipv6/output_core.c | 28 ++++++++++++++++++++++------
> net/ipv6/udp_offload.c | 10 +++++++++-
> 7 files changed, 65 insertions(+), 51 deletions(-)
>
> --
> 1.9.3
^ permalink raw reply
* [PATCH] net: sctp: Deletion of an unnecessary check before the function call "kfree"
From: SF Markus Elfring @ 2015-01-31 17:15 UTC (permalink / raw)
To: David S. Miller, Neil Horman, Vlad Yasevich, linux-sctp, 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 18:10:03 +0100
The kfree() 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/sctp/associola.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index f791edd..3c2aefc 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -391,8 +391,7 @@ void sctp_association_free(struct sctp_association *asoc)
sctp_asconf_queue_teardown(asoc);
/* Free pending address space being deleted */
- if (asoc->asconf_addr_del_pending != NULL)
- kfree(asoc->asconf_addr_del_pending);
+ kfree(asoc->asconf_addr_del_pending);
/* AUTH - Free the endpoint shared keys */
sctp_auth_destroy_keys(&asoc->endpoint_shared_keys);
--
2.2.2
^ permalink raw reply related
* Re: [PATCH v2 3/3] Revert "drivers/net: Disable UFO through virtio"
From: Michael S. Tsirkin @ 2015-01-31 17:11 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: eric.dumazet, netdev, virtualization, hannes, ben
In-Reply-To: <1422646047-13168-4-git-send-email-vyasevic@redhat.com>
On Fri, Jan 30, 2015 at 02:27:27PM -0500, Vladislav Yasevich wrote:
> This reverts commit 3d0ad09412ffe00c9afa201d01effdb6023d09b4.
>
> Now that GSO functionality can correctly track if the fragment
> id has been selected and select a fragment id if necessary,
> we can re-enable UFO on tap/macvap and virtio devices.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/macvtap.c | 13 ++++++++-----
> drivers/net/tun.c | 19 ++++++++-----------
> drivers/net/virtio_net.c | 24 ++++++++++--------------
> 3 files changed, 26 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 0b86e46..919f4fc 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -80,7 +80,7 @@ static struct cdev macvtap_cdev;
> static const struct proto_ops macvtap_socket_ops;
>
> #define TUN_OFFLOADS (NETIF_F_HW_CSUM | NETIF_F_TSO_ECN | NETIF_F_TSO | \
> - NETIF_F_TSO6)
> + NETIF_F_TSO6 | NETIF_F_UFO)
> #define RX_OFFLOADS (NETIF_F_GRO | NETIF_F_LRO)
> #define TAP_FEATURES (NETIF_F_GSO | NETIF_F_SG)
>
> @@ -585,8 +585,6 @@ static int macvtap_skb_from_vnet_hdr(struct macvtap_queue *q,
> gso_type = SKB_GSO_TCPV6;
> break;
> case VIRTIO_NET_HDR_GSO_UDP:
> - pr_warn_once("macvtap: %s: using disabled UFO feature; please fix this program\n",
> - current->comm);
> gso_type = SKB_GSO_UDP;
> break;
> default:
> @@ -633,6 +631,8 @@ static void macvtap_skb_to_vnet_hdr(struct macvtap_queue *q,
> vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> else if (sinfo->gso_type & SKB_GSO_TCPV6)
> vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> + else if (sinfo->gso_type & SKB_GSO_UDP)
> + vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_UDP;
> else
> BUG();
> if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> @@ -962,6 +962,9 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> if (arg & TUN_F_TSO6)
> feature_mask |= NETIF_F_TSO6;
> }
> +
> + if (arg & TUN_F_UFO)
> + feature_mask |= NETIF_F_UFO;
> }
>
> /* tun/tap driver inverts the usage for TSO offloads, where
> @@ -972,7 +975,7 @@ static int set_offload(struct macvtap_queue *q, unsigned long arg)
> * When user space turns off TSO, we turn off GSO/LRO so that
> * user-space will not receive TSO frames.
> */
> - if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6))
> + if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO))
> features |= RX_OFFLOADS;
> else
> features &= ~RX_OFFLOADS;
> @@ -1087,7 +1090,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> case TUNSETOFFLOAD:
> /* let the user check for future flags */
> if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> - TUN_F_TSO_ECN))
> + TUN_F_TSO_ECN | TUN_F_UFO))
> return -EINVAL;
>
> rtnl_lock();
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 5ca42b7..10f9e40 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -186,7 +186,7 @@ struct tun_struct {
> struct net_device *dev;
> netdev_features_t set_features;
> #define TUN_USER_FEATURES (NETIF_F_HW_CSUM|NETIF_F_TSO_ECN|NETIF_F_TSO| \
> - NETIF_F_TSO6)
> + NETIF_F_TSO6|NETIF_F_UFO)
>
> int vnet_hdr_sz;
> int sndbuf;
> @@ -1176,18 +1176,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> break;
> case VIRTIO_NET_HDR_GSO_UDP:
> - {
> - static bool warned;
> -
> - if (!warned) {
> - warned = true;
> - netdev_warn(tun->dev,
> - "%s: using disabled UFO feature; please fix this program\n",
> - current->comm);
> - }
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> break;
> - }
> default:
> tun->dev->stats.rx_frame_errors++;
> kfree_skb(skb);
> @@ -1294,6 +1284,8 @@ static ssize_t tun_put_user(struct tun_struct *tun,
> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> else if (sinfo->gso_type & SKB_GSO_TCPV6)
> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> + else if (sinfo->gso_type & SKB_GSO_UDP)
> + gso.gso_type = VIRTIO_NET_HDR_GSO_UDP;
> else {
> pr_err("unexpected GSO type: "
> "0x%x, gso_size %d, hdr_len %d\n",
> @@ -1742,6 +1734,11 @@ static int set_offload(struct tun_struct *tun, unsigned long arg)
> features |= NETIF_F_TSO6;
> arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
> }
> +
> + if (arg & TUN_F_UFO) {
> + features |= NETIF_F_UFO;
> + arg &= ~TUN_F_UFO;
> + }
> }
>
> /* This gives the user a way to test for new features in future by
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 5ca9771..059fdf1 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -490,17 +490,8 @@ static void receive_buf(struct virtnet_info *vi, struct receive_queue *rq,
> skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> break;
> case VIRTIO_NET_HDR_GSO_UDP:
> - {
> - static bool warned;
> -
> - if (!warned) {
> - warned = true;
> - netdev_warn(dev,
> - "host using disabled UFO feature; please fix it\n");
> - }
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> break;
> - }
> case VIRTIO_NET_HDR_GSO_TCPV6:
> skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> break;
> @@ -888,6 +879,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
> hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> + else if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
> + hdr->hdr.gso_type = VIRTIO_NET_HDR_GSO_UDP;
> else
> BUG();
> if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
> @@ -1748,7 +1741,7 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GSO)) {
> - dev->hw_features |= NETIF_F_TSO
> + dev->hw_features |= NETIF_F_TSO | NETIF_F_UFO
> | NETIF_F_TSO_ECN | NETIF_F_TSO6;
> }
> /* Individual feature bits: what can host handle? */
> @@ -1758,9 +1751,11 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->hw_features |= NETIF_F_TSO6;
> if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_ECN))
> dev->hw_features |= NETIF_F_TSO_ECN;
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_HOST_UFO))
> + dev->hw_features |= NETIF_F_UFO;
>
> if (gso)
> - dev->features |= dev->hw_features & NETIF_F_ALL_TSO;
> + dev->features |= dev->hw_features & (NETIF_F_ALL_TSO|NETIF_F_UFO);
> /* (!csum && gso) case will be fixed by register_netdev() */
> }
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_CSUM))
> @@ -1798,7 +1793,8 @@ static int virtnet_probe(struct virtio_device *vdev)
> /* If we can receive ANY GSO packets, we must allocate large ones. */
> if (virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO4) ||
> virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_TSO6) ||
> - virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN))
> + virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_ECN) ||
> + virtio_has_feature(vdev, VIRTIO_NET_F_GUEST_UFO))
> vi->big_packets = true;
>
> if (virtio_has_feature(vdev, VIRTIO_NET_F_MRG_RXBUF))
> @@ -1994,9 +1990,9 @@ static struct virtio_device_id id_table[] = {
> static unsigned int features[] = {
> VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
> VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
> - VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6,
> + VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
> VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
> - VIRTIO_NET_F_GUEST_ECN,
> + VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
> VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
> VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> --
> 1.9.3
^ permalink raw reply
* Re: [PATCH v2 2/3] Revert "drivers/net, ipv6: Select IPv6 fragment idents for virtio UFO packets"
From: Michael S. Tsirkin @ 2015-01-31 17:11 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: eric.dumazet, netdev, virtualization, hannes, ben
In-Reply-To: <1422646047-13168-3-git-send-email-vyasevic@redhat.com>
On Fri, Jan 30, 2015 at 02:27:26PM -0500, Vladislav Yasevich wrote:
> This reverts commit 5188cd44c55db3e92cd9e77a40b5baa7ed4340f7.
>
> Now that GSO layer can track if fragment id has been selected
> and can allocate one if necessary, we don't need to do this in
> tap and macvtap. This reverts most of the code and only keeps
> the new ipv6 fragment id generation function that is still needed.
>
> Fixes: 3d0ad09412ff (drivers/net: Disable UFO through virtio)
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/macvtap.c | 3 ---
> drivers/net/tun.c | 6 +-----
> 2 files changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 7df2217..0b86e46 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -17,7 +17,6 @@
> #include <linux/fs.h>
> #include <linux/uio.h>
>
> -#include <net/ipv6.h>
> #include <net/net_namespace.h>
> #include <net/rtnetlink.h>
> #include <net/sock.h>
> @@ -589,8 +588,6 @@ static int macvtap_skb_from_vnet_hdr(struct macvtap_queue *q,
> pr_warn_once("macvtap: %s: using disabled UFO feature; please fix this program\n",
> current->comm);
> gso_type = SKB_GSO_UDP;
> - if (skb->protocol == htons(ETH_P_IPV6))
> - ipv6_proxy_select_ident(skb);
> break;
> default:
> return -EINVAL;
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 8c8dc16..5ca42b7 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -65,7 +65,6 @@
> #include <linux/nsproxy.h>
> #include <linux/virtio_net.h>
> #include <linux/rcupdate.h>
> -#include <net/ipv6.h>
> #include <net/net_namespace.h>
> #include <net/netns/generic.h>
> #include <net/rtnetlink.h>
> @@ -1167,8 +1166,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> break;
> }
>
> - skb_reset_network_header(skb);
> -
> if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> pr_debug("GSO!\n");
> switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> @@ -1189,8 +1186,6 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> current->comm);
> }
> skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> - if (skb->protocol == htons(ETH_P_IPV6))
> - ipv6_proxy_select_ident(skb);
> break;
> }
> default:
> @@ -1221,6 +1216,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> skb_shinfo(skb)->tx_flags |= SKBTX_SHARED_FRAG;
> }
>
> + skb_reset_network_header(skb);
> skb_probe_transport_header(skb, 0);
>
> rxhash = skb_get_hash(skb);
> --
> 1.9.3
^ permalink raw reply
* Re: [PATCH v2 1/3] ipv6: Select fragment id during UFO segmentation if not set.
From: Michael S. Tsirkin @ 2015-01-31 17:11 UTC (permalink / raw)
To: Vladislav Yasevich; +Cc: eric.dumazet, netdev, virtualization, hannes, ben
In-Reply-To: <1422646047-13168-2-git-send-email-vyasevic@redhat.com>
On Fri, Jan 30, 2015 at 02:27:25PM -0500, Vladislav Yasevich wrote:
> If the IPv6 fragment id has not been set and we perform
> fragmentation due to UFO, select a new fragment id.
> We now consider a fragment id of 0 as unset and if id selection
> process returns 0 (after all the pertrubations), we set it to
> 0x80000000, thus giving us ample space not to create collisions
> with the next packet we may have to fragment.
>
> When doing UFO integrity checking, we also select the
> fragment id if it has not be set yet. This is stored into
s/be/been/
> the skb_shinfo() thus allowing UFO to function correclty.
>
> This patch also removes duplicate fragment id generation code
> and moves ipv6_select_ident() into the header as it may be
> used during GSO.
>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> include/net/ipv6.h | 3 +++
> net/ipv6/ip6_output.c | 10 ++++------
> net/ipv6/output_core.c | 28 ++++++++++++++++++++++------
> net/ipv6/udp_offload.c | 10 +++++++++-
> 4 files changed, 38 insertions(+), 13 deletions(-)
>
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 4292929..9bf85d3 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -671,6 +671,9 @@ static inline int ipv6_addr_diff(const struct in6_addr *a1, const struct in6_add
> return __ipv6_addr_diff(a1, a2, sizeof(struct in6_addr));
> }
>
> +u32 __ipv6_select_ident(u32 hashrnd, struct in6_addr *dst,
> + struct in6_addr *src);
> +void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt);
> void ipv6_proxy_select_ident(struct sk_buff *skb);
>
> int ip6_dst_hoplimit(struct dst_entry *dst);
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index ce69a12..5dc91fe 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -537,17 +537,15 @@ static void ip6_copy_metadata(struct sk_buff *to, struct sk_buff *from)
> skb_copy_secmark(to, from);
> }
>
> -static void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
> +void ipv6_select_ident(struct frag_hdr *fhdr, struct rt6_info *rt)
> {
> static u32 ip6_idents_hashrnd __read_mostly;
> - u32 hash, id;
> + u32 id;
>
> net_get_random_once(&ip6_idents_hashrnd, sizeof(ip6_idents_hashrnd));
>
> - hash = __ipv6_addr_jhash(&rt->rt6i_dst.addr, ip6_idents_hashrnd);
> - hash = __ipv6_addr_jhash(&rt->rt6i_src.addr, hash);
> -
> - id = ip_idents_reserve(hash, 1);
> + id = __ipv6_select_ident(ip6_idents_hashrnd, &rt->rt6i_dst.addr,
> + &rt->rt6i_src.addr);
> fhdr->identification = htonl(id);
> }
>
> diff --git a/net/ipv6/output_core.c b/net/ipv6/output_core.c
> index 97f41a3..ceb857d 100644
> --- a/net/ipv6/output_core.c
> +++ b/net/ipv6/output_core.c
> @@ -9,6 +9,24 @@
> #include <net/addrconf.h>
> #include <net/secure_seq.h>
>
> +u32 __ipv6_select_ident(u32 hashrnd, struct in6_addr *dst, struct in6_addr *src)
> +{
> + u32 hash, id;
> +
> + hash = __ipv6_addr_jhash(dst, hashrnd);
> + hash = __ipv6_addr_jhash(src, hash);
> +
> + /* Treat id of 0 as unset and if we get 0 back from ip_idents_reserve,
> + * set the hight order instead thus minimizing possible future
> + * collisions.
> + */
> + id = ip_idents_reserve(hash, 1);
> + if (unlikely(!id))
> + id = 1 << 31;
> +
> + return id;
> +}
> +
> /* This function exists only for tap drivers that must support broken
> * clients requesting UFO without specifying an IPv6 fragment ID.
> *
> @@ -22,7 +40,7 @@ void ipv6_proxy_select_ident(struct sk_buff *skb)
> static u32 ip6_proxy_idents_hashrnd __read_mostly;
> struct in6_addr buf[2];
> struct in6_addr *addrs;
> - u32 hash, id;
> + u32 id;
>
> addrs = skb_header_pointer(skb,
> skb_network_offset(skb) +
> @@ -34,11 +52,9 @@ void ipv6_proxy_select_ident(struct sk_buff *skb)
> net_get_random_once(&ip6_proxy_idents_hashrnd,
> sizeof(ip6_proxy_idents_hashrnd));
>
> - hash = __ipv6_addr_jhash(&addrs[1], ip6_proxy_idents_hashrnd);
> - hash = __ipv6_addr_jhash(&addrs[0], hash);
> -
> - id = ip_idents_reserve(hash, 1);
> - skb_shinfo(skb)->ip6_frag_id = htonl(id);
> + id = __ipv6_select_ident(ip6_proxy_idents_hashrnd,
> + &addrs[1], &addrs[0]);
> + skb_shinfo(skb)->ip6_frag_id = id;
> }
> EXPORT_SYMBOL_GPL(ipv6_proxy_select_ident);
>
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index b6aa8ed..a562769 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -52,6 +52,10 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
>
> skb_shinfo(skb)->gso_segs = DIV_ROUND_UP(skb->len, mss);
>
> + /* Set the IPv6 fragment id if not set yet */
> + if (!skb_shinfo(skb)->ip6_frag_id)
> + ipv6_proxy_select_ident(skb);
> +
> segs = NULL;
> goto out;
> }
> @@ -108,7 +112,11 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> fptr = (struct frag_hdr *)(skb_network_header(skb) + unfrag_ip6hlen);
> fptr->nexthdr = nexthdr;
> fptr->reserved = 0;
> - fptr->identification = skb_shinfo(skb)->ip6_frag_id;
> + if (skb_shinfo(skb)->ip6_frag_id)
> + fptr->identification = skb_shinfo(skb)->ip6_frag_id;
> + else
> + ipv6_select_ident(fptr,
> + (struct rt6_info *)skb_dst(skb));
>
> /* Fragment the skb. ipv6 header and the remaining fields of the
> * fragment header are updated in ipv6_gso_segment()
> --
> 1.9.3
^ 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