* Re: [PATCH v2 1/6] net-PPP: Replacement of a printk() call by pr_warn() in mppe_rekey()
From: SF Markus Elfring @ 2014-12-04 22:27 UTC (permalink / raw)
To: Joe Perches
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <1417731809.2721.17.camel@perches.com>
>> diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
> []
>> @@ -172,9 +172,8 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
>> setup_sg(sg_in, state->sha1_digest, state->keylen);
>> setup_sg(sg_out, state->session_key, state->keylen);
>> if (crypto_blkcipher_encrypt(&desc, sg_out, sg_in,
>> - state->keylen) != 0) {
>> - printk(KERN_WARNING "mppe_rekey: cipher_encrypt failed\n");
>> - }
>> + state->keylen) != 0)
>> + pr_warn("mppe_rekey: cipher_encrypt failed\n");
>
> It's generally nicer to replace embedded function names
> with "%s: ", __func__
>
> pr_warn("%s: cipher_encrypt failed\n", __func__);
Do you want that I send a third patch series for the fine-tuning of these parameters?
Regards,
Martkus
^ permalink raw reply
* Re: [PATCH v2 1/6] net-PPP: Replacement of a printk() call by pr_warn() in mppe_rekey()
From: Joe Perches @ 2014-12-04 22:23 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet,
LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DBDE.7040604@users.sourceforge.net>
On Thu, 2014-12-04 at 23:10 +0100, SF Markus Elfring wrote:
> The mppe_rekey() function contained a few update candidates.
> * Curly brackets were still used around a single function call "printk".
> * Unwanted space characters
>
> Let us improve these implementation details according to the current Linux
> coding style convention.
trivia:
> diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
[]
> @@ -172,9 +172,8 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
> setup_sg(sg_in, state->sha1_digest, state->keylen);
> setup_sg(sg_out, state->session_key, state->keylen);
> if (crypto_blkcipher_encrypt(&desc, sg_out, sg_in,
> - state->keylen) != 0) {
> - printk(KERN_WARNING "mppe_rekey: cipher_encrypt failed\n");
> - }
> + state->keylen) != 0)
> + pr_warn("mppe_rekey: cipher_encrypt failed\n");
It's generally nicer to replace embedded function names
with "%s: ", __func__
pr_warn("%s: cipher_encrypt failed\n", __func__);
^ permalink raw reply
* [PATCH v2 6/6] net-PPP: Delete another unnecessary assignment in mppe_alloc()
From: SF Markus Elfring @ 2014-12-04 22:20 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:42:30 +0100
The data structure element "sha1" was assigned a null pointer by the
mppe_alloc() after a function call "crypto_alloc_hash" failed.
It was determined that this element was not accessed by the implementation
of the crypto_free_blkcipher() function.
Let us delete it from the affected implementation because the element "sha1"
will not be accessible outside the function after the detected
allocation failure.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ppp/ppp_mppe.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index b7db4b1..32cb054 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -208,10 +208,8 @@ static void *mppe_alloc(unsigned char *options, int optlen)
goto out_free;
state->sha1 = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC);
- if (IS_ERR(state->sha1)) {
- state->sha1 = NULL;
+ if (IS_ERR(state->sha1))
goto out_free_blkcipher;
- }
digestsize = crypto_hash_digestsize(state->sha1);
if (digestsize < MPPE_MAX_KEY_LEN)
--
2.1.3
^ permalink raw reply related
* [PATCH v2 5/6] net-PPP: Delete an unnecessary assignment in mppe_alloc()
From: SF Markus Elfring @ 2014-12-04 22:18 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:33:34 +0100
The data structure element "arc4" was assigned a null pointer by the
mppe_alloc() function if a previous function call "crypto_alloc_blkcipher"
failed. This assignment became unnecessary with previous source
code adjustments.
Let us delete it from the affected implementation because the element "arc4"
will not be accessible outside the function after the detected
allocation failure.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ppp/ppp_mppe.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index c82198f..b7db4b1 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -204,10 +204,8 @@ static void *mppe_alloc(unsigned char *options, int optlen)
state->arc4 = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
- if (IS_ERR(state->arc4)) {
- state->arc4 = NULL;
+ if (IS_ERR(state->arc4))
goto out_free;
- }
state->sha1 = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(state->sha1)) {
--
2.1.3
^ permalink raw reply related
* [PATCH v2 4/6] net-PPP: Less function calls in mppe_alloc() after error detection
From: SF Markus Elfring @ 2014-12-04 22:16 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:30:20 +0100
The functions crypto_free_blkcipher((), crypto_free_hash() and kfree() could be
called in some cases by the mppe_alloc() function during error handling even
if the passed data structure element contained still a null pointer.
This implementation detail could be improved by adjustments for jump labels.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ppp/ppp_mppe.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 94ff216..c82198f 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -196,11 +196,11 @@ static void *mppe_alloc(unsigned char *options, int optlen)
if (optlen != CILEN_MPPE + sizeof(state->master_key) ||
options[0] != CI_MPPE || options[1] != CILEN_MPPE)
- goto out;
+ return NULL;
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (state == NULL)
- goto out;
+ return NULL;
state->arc4 = crypto_alloc_blkcipher("ecb(arc4)", 0, CRYPTO_ALG_ASYNC);
@@ -212,16 +212,16 @@ static void *mppe_alloc(unsigned char *options, int optlen)
state->sha1 = crypto_alloc_hash("sha1", 0, CRYPTO_ALG_ASYNC);
if (IS_ERR(state->sha1)) {
state->sha1 = NULL;
- goto out_free;
+ goto out_free_blkcipher;
}
digestsize = crypto_hash_digestsize(state->sha1);
if (digestsize < MPPE_MAX_KEY_LEN)
- goto out_free;
+ goto out_free_hash;
state->sha1_digest = kmalloc(digestsize, GFP_KERNEL);
if (!state->sha1_digest)
- goto out_free;
+ goto out_free_hash;
/* Save keys. */
memcpy(state->master_key, &options[CILEN_MPPE],
@@ -236,14 +236,12 @@ static void *mppe_alloc(unsigned char *options, int optlen)
return (void *)state;
+out_free_hash:
+ crypto_free_hash(state->sha1);
+out_free_blkcipher:
+ crypto_free_blkcipher(state->arc4);
out_free:
- kfree(state->sha1_digest);
- if (state->sha1)
- crypto_free_hash(state->sha1);
- if (state->arc4)
- crypto_free_blkcipher(state->arc4);
kfree(state);
-out:
return NULL;
}
--
2.1.3
^ permalink raw reply related
* [PATCH v2 3/6] net-PPP: Deletion of unnecessary checks before the function call "kfree"
From: SF Markus Elfring @ 2014-12-04 22:15 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:22:23 +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>
---
drivers/net/ppp/ppp_mppe.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index b80af29..94ff216 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -237,8 +237,7 @@ static void *mppe_alloc(unsigned char *options, int optlen)
return (void *)state;
out_free:
- if (state->sha1_digest)
- kfree(state->sha1_digest);
+ kfree(state->sha1_digest);
if (state->sha1)
crypto_free_hash(state->sha1);
if (state->arc4)
@@ -255,8 +254,7 @@ static void mppe_free(void *arg)
{
struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
if (state) {
- if (state->sha1_digest)
- kfree(state->sha1_digest);
+ kfree(state->sha1_digest);
if (state->sha1)
crypto_free_hash(state->sha1);
if (state->arc4)
--
2.1.3
^ permalink raw reply related
* [PATCH v2 2/6] net-PPP: Fix indentation
From: SF Markus Elfring @ 2014-12-04 22:13 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:15:20 +0100
The implementations of the functions "mppe_alloc" and "mppe_free" contained
unwanted space characters.
Let us improve the indentation according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ppp/ppp_mppe.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 84b7bce..b80af29 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -236,15 +236,15 @@ static void *mppe_alloc(unsigned char *options, int optlen)
return (void *)state;
- out_free:
- if (state->sha1_digest)
+out_free:
+ if (state->sha1_digest)
kfree(state->sha1_digest);
- if (state->sha1)
+ if (state->sha1)
crypto_free_hash(state->sha1);
- if (state->arc4)
+ if (state->arc4)
crypto_free_blkcipher(state->arc4);
- kfree(state);
- out:
+ kfree(state);
+out:
return NULL;
}
@@ -255,13 +255,13 @@ static void mppe_free(void *arg)
{
struct ppp_mppe_state *state = (struct ppp_mppe_state *) arg;
if (state) {
- if (state->sha1_digest)
- kfree(state->sha1_digest);
- if (state->sha1)
- crypto_free_hash(state->sha1);
- if (state->arc4)
- crypto_free_blkcipher(state->arc4);
- kfree(state);
+ if (state->sha1_digest)
+ kfree(state->sha1_digest);
+ if (state->sha1)
+ crypto_free_hash(state->sha1);
+ if (state->arc4)
+ crypto_free_blkcipher(state->arc4);
+ kfree(state);
}
}
--
2.1.3
^ permalink raw reply related
* [PATCH v2 1/6] net-PPP: Replacement of a printk() call by pr_warn() in mppe_rekey()
From: SF Markus Elfring @ 2014-12-04 22:10 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5480DA32.8000201@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 18:28:52 +0100
The mppe_rekey() function contained a few update candidates.
* Curly brackets were still used around a single function call "printk".
* Unwanted space characters
Let us improve these implementation details according to the current Linux
coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ppp/ppp_mppe.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ppp/ppp_mppe.c b/drivers/net/ppp/ppp_mppe.c
index 911b216..84b7bce 100644
--- a/drivers/net/ppp/ppp_mppe.c
+++ b/drivers/net/ppp/ppp_mppe.c
@@ -172,9 +172,8 @@ static void mppe_rekey(struct ppp_mppe_state * state, int initial_key)
setup_sg(sg_in, state->sha1_digest, state->keylen);
setup_sg(sg_out, state->session_key, state->keylen);
if (crypto_blkcipher_encrypt(&desc, sg_out, sg_in,
- state->keylen) != 0) {
- printk(KERN_WARNING "mppe_rekey: cipher_encrypt failed\n");
- }
+ state->keylen) != 0)
+ pr_warn("mppe_rekey: cipher_encrypt failed\n");
} else {
memcpy(state->session_key, state->sha1_digest, state->keylen);
}
--
2.1.3
^ permalink raw reply related
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Thomas Graf @ 2014-12-04 22:07 UTC (permalink / raw)
To: Jiri Pirko
Cc: Eric W. Biederman, netdev, davem, nhorman, andy, dborkman,
ogerlitz, jesse, pshelar, azhou, ben, stephen, jeffrey.t.kirsher,
vyasevic, xiyou.wangcong, john.r.fastabend, edumazet, jhs,
sfeldma, f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, b
In-Reply-To: <20141204211041.GN1861@nanopsycho.orion>
On 12/04/14 at 10:10pm, Jiri Pirko wrote:
> Thu, Dec 04, 2014 at 09:55:07PM CET, ebiederm@xmission.com wrote:
> >My intuition says we want something like ifindex, but I am not at all
> >certain how switch id is planned to be used. Given that it is single
> >box I don't expect you are sending it out over the wire.
>
> No, it is not to be send out.
We discussed this before and stated we would fix this as soon as it's
merged. Let's just define a simple and quick switch id prefix generator
ala ifindex which serves as a unique prefix to all switch ids.
I have time early next week if Jiri doesn't.
^ permalink raw reply
* [PATCH v2 0/6] net-PPP: Deletion of a few unnecessary checks
From: SF Markus Elfring @ 2014-12-04 22:03 UTC (permalink / raw)
To: Sergei Shtylyov, Paul Mackerras, linux-ppp, netdev, Eric Dumazet
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <547CA157.1080401@cogentembedded.com>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 Dec 2014 22:50:28 +0100
Further update suggestions were taken into account before and after a patch
was applied from static source code analysis.
Markus Elfring (6):
Replacement of a printk() call by pr_warn() in mppe_rekey()
Fix indentation
Deletion of unnecessary checks before the function call "kfree"
Less function calls in mppe_alloc() after error detection
Delete an unnecessary assignment in mppe_alloc()
Delete another unnecessary assignment in mppe_alloc()
drivers/net/ppp/ppp_mppe.c | 49 +++++++++++++++++++---------------------------
1 file changed, 20 insertions(+), 29 deletions(-)
--
2.1.3
^ permalink raw reply
* Re: [PATCH iproute2] ip monitor: Fixed printing timestamp few times
From: Vadim Kochan @ 2014-12-04 21:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev@vger.kernel.org
In-Reply-To: <20141203174153.GA12034@angus-think.wlc.globallogic.com>
Hi Stephen,
Seems that problem is that by default ipmonitor subscribes to the all
RTNL groups except RTMGRP_TC:
groups = ~RTMGRP_TC;
And I assume that other netlink messages can also cause such
Timestamping w/o event message ...
So I think there should be another way than ~RTMGR_TC, and this big IF
in accept_msg can be removed.
Regards,
Vadim
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Eric W. Biederman @ 2014-12-04 21:24 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl, hemal
In-Reply-To: <20141204211041.GN1861@nanopsycho.orion>
Jiri Pirko <jiri@resnulli.us> writes:
>>No. phys_port_id is not broken in the same way, and phys_port_id does
>>not have the same set of properties.
>>
>>phys_port_id's in practice all have an IEEE prefix that identifies the
>>manufacturer and a manufacture assigned serial number. Aka a mac
>>address or a EUID-64. What the mlx4 ethernet driver is doing retunring
>>a 64bit EUID-64 I don't know. If there are problems in the worst
>>case issues with phys_port_id are fixable by simple driver tweaks,
>>because fundamentally we are working with globally uniuqe identifiers.
>>Well globally unique baring manufacturing bugs in eeproms.
>
> Well the fact that phys_post_id's are now implemented mostly by putting
> mac into it does not mean that other drivers cannot do it differently.
> So once again, phys_port_id and phys_switch_id are the same in this
> matter.
No exclusively implemented that way.
And yes other drivers can implement bugs, that doesn't mean those bugs
will be toleraged and won't break userspace.
>>I agree with you that the switch id concept can be saved. But I think
>>we should fix switch id before we export it to userspace so we don't
>>have to break userspace later.
>>
>>My intuition says we want something like ifindex, but I am not at all
>>certain how switch id is planned to be used. Given that it is single
>>box I don't expect you are sending it out over the wire.
>
> No, it is not to be send out.
>
>>
>>*shrug*
>>
>>Why does switch id need to be persistent? Why can't switch id be
>>property like ifindex?
>
> Well I can imagine that multiple ports of the same switch chip could be
> passed through to the virtual machines (similar to SR-IOV pf/vf).
In that case you do need something that is globally unique. Because
if I migrate your virtual machine onto a different physical machine
seeing the same switch id and I might make the mistaken assumption
that I am remain on the same switch if the ids are not global.
>>What are the actual requirements.
>
>
> They are actually very similar to phys_port_id. Therefore I made that
> the same.
Then please for rocket use a non-buggy implementation with a globally
unique id.
Given your descriptions of the requirements I can't see how any other
implementation isn't buggy.
Eric
^ permalink raw reply
* Re: [patch] ipvs: uninitialized data with IP_VS_IPV6
From: Julian Anastasov @ 2014-12-04 21:19 UTC (permalink / raw)
To: Dan Carpenter
Cc: Wensong Zhang, Simon Horman, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, netdev, lvs-devel,
netfilter-devel, coreteam, kernel-janitors
In-Reply-To: <20141203101213.GC29583@mwanda>
Hello,
On Wed, 3 Dec 2014, Dan Carpenter wrote:
> The app_tcp_pkt_out() function expects "*diff" to be set and ends up
> using uninitialized data if CONFIG_IP_VS_IPV6 is turned on.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> This bug is very old.
I guess ip_vs_ftp_in() needs the same fix?
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCHv3 net] i40e: Implement ndo_gso_check()
From: Joe Stringer @ 2014-12-04 21:15 UTC (permalink / raw)
To: Tom Herbert
Cc: Linux Netdev List, LKML, Jesse Gross, Shannon Nelson,
Brandeburg, Jesse, Jeff Kirsher, linux.nics
In-Reply-To: <CA+mtBx_BiJ526jop1tLhx1ji4beRufQRhiN9nfFBWLnr25DMww@mail.gmail.com>
On 4 December 2014 at 12:17, Tom Herbert <therbert@google.com> wrote:
> On Thu, Dec 4, 2014 at 10:39 AM, Joe Stringer <joestringer@nicira.com> wrote:
>> ndo_gso_check() was recently introduced to allow NICs to report the
>> offloading support that they have on a per-skb basis. Add an
>> implementation for this driver which checks for IPIP, GRE, UDP tunnels.
>>
>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>> ---
>> v3: Drop IPIP and GRE (no driver support even though hw supports it).
>> Check for UDP outer protocol for UDP tunnels.
>> v2: Expand to include IP in IP and IPv4/IPv6 inside GRE/UDP tunnels.
>> Add MAX_INNER_LENGTH (as 80).
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_main.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index c3a7f4a..0d6493a 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -7447,6 +7447,31 @@ static int i40e_ndo_fdb_dump(struct sk_buff *skb,
>>
>> #endif /* USE_DEFAULT_FDB_DEL_DUMP */
>> #endif /* HAVE_FDB_OPS */
>> +static bool i40e_gso_check(struct sk_buff *skb, struct net_device *dev)
>> +{
>> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) {
>> + unsigned char *ihdr;
>> +
>> + if (skb->protocol != IPPROTO_UDP ||
>> + skb->inner_protocol_type != ENCAP_TYPE_ETHER)
>> + return false;
>> +
>> + if (skb->inner_protocol == htons(ETH_P_TEB))
>> + ihdr = skb_inner_mac_header(skb);
>> + else if (skb->inner_protocol == htons(ETH_P_IP) ||
>> + skb->inner_protocol == htons(ETH_P_IPV6))
>> + ihdr = skb_inner_network_header(skb);
>> + else
>> + return false;
>> +
>
> Wow, this is getting complicated! :-( It's not clear that the protocol
> specific checks are needed here since it looks like the header length
> is being passed to the device later on. Also, I think we need
> skb_inner_mac_header(skb) - skb_transport_header(skb) to always work
> to give the length of the encapsulation headers (in case there is no
> real inner mac header, then that offset should be for the network
> header).
>
> So would a simple check like this work:
>
> if (skb->encapsulation &&
> (skb_inner_mac_header(skb) - skb_transport_header(skb)) >
> MAX_TUNNEL_HDR_LEN)
> return false;
Ah, I didn't realise that the inner_mac_header() would be equivalent
to inner network when inner mac doesn't exist. This is much tidier,
thanks Tom.
^ permalink raw reply
* Re: [linux-nics] [PATCHv3 net] i40e: Implement ndo_gso_check()
From: Joe Stringer @ 2014-12-04 21:12 UTC (permalink / raw)
To: Jeff Kirsher
Cc: Sergei Shtylyov, Linux Netdev List, linux.nics, Jesse Gross,
Linux Kernel, Tom Herbert
In-Reply-To: <1417720816.2404.11.camel@jtkirshe-mobl>
>> > +#define MAX_TUNNEL_HDR_LEN 80
>>
>> I'd #define this just above the function, if not at the start of the file...
Right, the style for most of this file is to place the #define like
this above the function. I'll do that.
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Jiri Pirko @ 2014-12-04 21:10 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl, hemal
In-Reply-To: <87388vw2xg.fsf@x220.int.ebiederm.org>
Thu, Dec 04, 2014 at 09:55:07PM CET, ebiederm@xmission.com wrote:
>Jiri Pirko <jiri@resnulli.us> writes:
>
>> Thu, Dec 04, 2014 at 09:06:14PM CET, ebiederm@xmission.com wrote:
>>>ebiederm@xmission.com (Eric W. Biederman) writes:
>>>
>>>> Jiri Pirko <jiri@resnulli.us> writes:
>>>>
>>>>>>So this id needs to be globally unique?
>>>>>
>>>>> No. It is enough to be unique within a single system. It serves for no
>>>>> more than to find out 2 ids are same or not, no other info value.
>>>>>
>>>>> So when the drivers uses sane ids (like mac for example, or in case of
>>>>> rocker an id which is passed by qemu command line), the chances of
>>>>> collision are very very close to none (never say never).
>>>
>>>Thinking about what you said a little more.
>>>
>>>Two different sources of persistent numbers picking numbers by
>>>completely different algorithms can give you no assurance that you don't
>>>produce conflicts.
>>>
>>>The switch id as desisgned can not work.
>>>
>>>There are expected to be between 2**36 to 2**40 devices in this world.
>>>Your first switch id is a 64it number. At the very best by the birthday
>>>pardox predicts there will be a conflict ever 2**32 devices or between
>>>2**4 and 2**8 devices in the world with conflicts. If the ids are not
>>>randomly distributed (which they won't be) things could easily be much
>>>much worse.
>>>
>>>That is just good enough the code could get out there and run for years
>>>before you have the nightmare of having to fix all of userspace. That
>>>is a nightmare no one needs.
>>>
>>>So please remove this broken code, and this broken concept from the
>>>kernel and go back to the drawing board.
>>
>> In that case the phys port id is broken in the same way. Let's rather
>> think about how to avoid conflicts for both. Given the fact the
>> conflicts should be avoided only on a single baremetal, that should be
>> doable (for (bad) example using driver name mixed with driver created
>> id).
>
>No. phys_port_id is not broken in the same way, and phys_port_id does
>not have the same set of properties.
>
>phys_port_id's in practice all have an IEEE prefix that identifies the
>manufacturer and a manufacture assigned serial number. Aka a mac
>address or a EUID-64. What the mlx4 ethernet driver is doing retunring
>a 64bit EUID-64 I don't know. If there are problems in the worst
>case issues with phys_port_id are fixable by simple driver tweaks,
>because fundamentally we are working with globally uniuqe identifiers.
>Well globally unique baring manufacturing bugs in eeproms.
Well the fact that phys_post_id's are now implemented mostly by putting
mac into it does not mean that other drivers cannot do it differently.
So once again, phys_port_id and phys_switch_id are the same in this
matter.
>
>
>
>I agree with you that the switch id concept can be saved. But I think
>we should fix switch id before we export it to userspace so we don't
>have to break userspace later.
>
>My intuition says we want something like ifindex, but I am not at all
>certain how switch id is planned to be used. Given that it is single
>box I don't expect you are sending it out over the wire.
No, it is not to be send out.
>
>*shrug*
>
>Why does switch id need to be persistent? Why can't switch id be
>property like ifindex?
Well I can imagine that multiple ports of the same switch chip could be
passed through to the virtual machines (similar to SR-IOV pf/vf).
>
>What are the actual requirements.
They are actually very similar to phys_port_id. Therefore I made that
the same.
>
>Eric
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Eric W. Biederman @ 2014-12-04 20:55 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl, hemal
In-Reply-To: <20141204202742.GM1861@nanopsycho.orion>
Jiri Pirko <jiri@resnulli.us> writes:
> Thu, Dec 04, 2014 at 09:06:14PM CET, ebiederm@xmission.com wrote:
>>ebiederm@xmission.com (Eric W. Biederman) writes:
>>
>>> Jiri Pirko <jiri@resnulli.us> writes:
>>>
>>>>>So this id needs to be globally unique?
>>>>
>>>> No. It is enough to be unique within a single system. It serves for no
>>>> more than to find out 2 ids are same or not, no other info value.
>>>>
>>>> So when the drivers uses sane ids (like mac for example, or in case of
>>>> rocker an id which is passed by qemu command line), the chances of
>>>> collision are very very close to none (never say never).
>>
>>Thinking about what you said a little more.
>>
>>Two different sources of persistent numbers picking numbers by
>>completely different algorithms can give you no assurance that you don't
>>produce conflicts.
>>
>>The switch id as desisgned can not work.
>>
>>There are expected to be between 2**36 to 2**40 devices in this world.
>>Your first switch id is a 64it number. At the very best by the birthday
>>pardox predicts there will be a conflict ever 2**32 devices or between
>>2**4 and 2**8 devices in the world with conflicts. If the ids are not
>>randomly distributed (which they won't be) things could easily be much
>>much worse.
>>
>>That is just good enough the code could get out there and run for years
>>before you have the nightmare of having to fix all of userspace. That
>>is a nightmare no one needs.
>>
>>So please remove this broken code, and this broken concept from the
>>kernel and go back to the drawing board.
>
> In that case the phys port id is broken in the same way. Let's rather
> think about how to avoid conflicts for both. Given the fact the
> conflicts should be avoided only on a single baremetal, that should be
> doable (for (bad) example using driver name mixed with driver created
> id).
No. phys_port_id is not broken in the same way, and phys_port_id does
not have the same set of properties.
phys_port_id's in practice all have an IEEE prefix that identifies the
manufacturer and a manufacture assigned serial number. Aka a mac
address or a EUID-64. What the mlx4 ethernet driver is doing retunring
a 64bit EUID-64 I don't know. If there are problems in the worst
case issues with phys_port_id are fixable by simple driver tweaks,
because fundamentally we are working with globally uniuqe identifiers.
Well globally unique baring manufacturing bugs in eeproms.
I agree with you that the switch id concept can be saved. But I think
we should fix switch id before we export it to userspace so we don't
have to break userspace later.
My intuition says we want something like ifindex, but I am not at all
certain how switch id is planned to be used. Given that it is single
box I don't expect you are sending it out over the wire.
*shrug*
Why does switch id need to be persistent? Why can't switch id be
property like ifindex?
What are the actual requirements.
Eric
^ permalink raw reply
* Re: [patch iproute2 4/6] bridge/link: add new offload hwmode swdev
From: Scott Feldman @ 2014-12-04 20:55 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Florian Fainelli, Roopa Prabhu,
John Linville
In-Reply-To: <54806042.5010007@mojatatu.com>
On Thu, Dec 4, 2014 at 5:23 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 12/04/14 03:57, Jiri Pirko wrote:
>>
>> From: Scott Feldman <sfeldma@gmail.com>
>>
>> To support full-featured switch devices offloading bridge funtionality,
>> add new hwmode 'swdev'. Like 'vepa' and 'veb', 'swdev' indicated bridge
>> port functionality is being offloaded to hardware.
>>
>
> Unhappy with the name swdev ..
> Ok, go ahead and beat up on me.
Lack of creativity, I know. But, I doubt anyone is confused by the name.
Hopefully it's short-lived and replace with what Roopa comes up with
for switch port offload flag.
^ permalink raw reply
* Re: [patch iproute2 0/6] iproute2: add changes for switchdev
From: Scott Feldman @ 2014-12-04 20:49 UTC (permalink / raw)
To: Roopa Prabhu
Cc: Jiri Pirko, Netdev, David S. Miller, nhorman@tuxdriver.com,
Andy Gospodarek, Thomas Graf, dborkman@redhat.com,
ogerlitz@mellanox.com, jesse@nicira.com, pshelar@nicira.com,
azhou@nicira.com, ben@decadent.org.uk, stephen@networkplumber.org,
Kirsher, Jeffrey T, vyasevic@redhat.com, Cong Wang,
Fastabend, John R, Eric Dumazet, Jamal Hadi Salim,
Florian Fainelli, John Linville
In-Reply-To: <5480921D.60108@cumulusnetworks.com>
On Thu, Dec 4, 2014 at 8:55 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On 12/4/14, 8:04 AM, Jiri Pirko wrote:
>>
>> Thu, Dec 04, 2014 at 03:45:44PM CET, roopa@cumulusnetworks.com wrote:
>>>
>>> On 12/4/14, 6:34 AM, Jiri Pirko wrote:
>>>>
>>>> Thu, Dec 04, 2014 at 03:26:50PM CET, roopa@cumulusnetworks.com wrote:
>>>>>
>>>>> On 12/4/14, 12:57 AM, Jiri Pirko wrote:
>>>>>>
>>>>>> Jiri Pirko (1):
>>>>>> iproute2: ipa: show switch id
>>>>>>
>>>>>> Scott Feldman (5):
>>>>>> bridge/fdb: fix statistics output spacing
>>>>>> bridge/fdb: add flag/indication for FDB entry synced from offload
>>>>>> device
>>>>>> bridge/link: add new offload hwmode swdev
>>>>>
>>>>> Ack to most patches but nack on this one. The todo list still has a
>>>>> note to
>>>>> revist the flag to indicate switchdev offloads.
>>>>> Exposing this to userspace does not help that.
>>>>
>>>> Hmm, note that this is already exposed to userspace, this patchset is
>>>> for iproute2 (userspace tool).
>>>
>>> hmmm, all feedback on the switchdev patches seemed to indicate we can
>>> change
>>> this later.
>>> I don't see swdev mode being used in the kernel anywhere today.
>>
>> Well, it is, in rocker:
>> $ git grep BRIDGE_MODE_SWDEV
>> drivers/net/ethernet/rocker/rocker.c: if (mode !=
>> BRIDGE_MODE_SWDEV)
>> drivers/net/ethernet/rocker/rocker.c: u16 mode = BRIDGE_MODE_SWDEV;
>> include/uapi/linux/if_bridge.h:#define BRIDGE_MODE_SWDEV 2 /*
>> Full switch device offload */
>
>
> The problem is rocker is not the only one who is going to be using this. And
> so, we need something that fits everybody.
> And i am not going to make my user set a mode for him to enable offload to
> hw.
>
>>
>>> I will send a patch to remove it. Its still in net-next and so can be
>>> changed
>>> ?.
>>> I was going to resend my patch to introduce a common offload flag for all
>>> link objects.
>>> It would be nice if all of them had a consistent flag to indicate hw
>>> offload
>>> and iproute2 could display the same flag for all.
>>> Including bonds and vxlan's.
>>
>> I do not understand the connection with BRIDGE_MODE_SWDEV. We discussed
>> this already. BRIDGE_MODE_SWDEV is a bridge mode, similar to for example
>> BRIDGE_MODE_VEPA and makes perfect sense to have it.
>
> I dont think everybody acked it. But it went in with a note saying that it
> can be changed.
I thought that was the plan: this new mode goes in now for net-next
and iproute2, and you would supply follow up patch for each to move to
your switch port flag. That will give us time to review your work
without have net-next and iproute2 out-of-sync.
>>
>>
>> How vxlan and bonds come into the mixture, that is a puzzler for me.
>> Maybe I have to see patches.
>
>
> I had posted a version of the patch previously:
> http://www.spinics.net/lists/netdev/msg305472.html
>
> I have a v2 patch in my stack which does not touch the netlink header.
> But in the past hour, i have been thinking about it some more. Do we really
> need this set by the user ?. In my use case i don't need it.
Look at how iproute2 figures out if SELF should be set or not. It's
only set if hwmode is set, otherwise it defaults to MASTER. So with
SWDEV a new hwmode, we can push settings (learning, leraning_sync) to
port with SELF set. It's probably not an ideal arrangement having to
set hwmode each time, but this was the low-touch change to iproute2 to
push port settings.
I'm hoping your new patch will kind of straighten this all out. But
you've got extra work to make sure backward compat with older iproute2
still works, including this weirdness around hwmode.
>
> We do need a feature flag (or net_device_flags), but it does not need to be
> set by the user explicitly.
> This flag can be set by the switch port driver on the switch ports. And the
> logical device: bridge/bond/vxlan
> can inherit it from the port. There was a need of a flag in some usecases,
> to control offloading of specific bridge port flags
> to hw/sw (example learning in hw or sw). example patch:
> https://patchwork.ozlabs.org/patch/413211/
>
> I will post something today.
Can you include matching iproute2 changes? (Assuming you'll building
on top of what's already in net-next and this iproute2 set Jiri sent
out). It's helpful to see the iproute2 changes to see what the new
cmd structure is and how legacy is handled.
-scott
^ permalink raw reply
* [patch net-next] net: sched: cls: remove unused op put from tcf_proto_ops
From: Jiri Pirko @ 2014-12-04 20:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs
It is never called and implementations are void. So just remove it.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
include/net/sch_generic.h | 1 -
net/sched/cls_basic.c | 5 -----
net/sched/cls_bpf.c | 5 -----
net/sched/cls_cgroup.c | 5 -----
net/sched/cls_flow.c | 5 -----
net/sched/cls_fw.c | 5 -----
net/sched/cls_route.c | 5 -----
net/sched/cls_rsvp.h | 5 -----
net/sched/cls_tcindex.c | 8 --------
net/sched/cls_u32.c | 5 -----
10 files changed, 49 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index d17ed6f..3d282cb 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -219,7 +219,6 @@ struct tcf_proto_ops {
void (*destroy)(struct tcf_proto*);
unsigned long (*get)(struct tcf_proto*, u32 handle);
- void (*put)(struct tcf_proto*, unsigned long);
int (*change)(struct net *net, struct sk_buff *,
struct tcf_proto*, unsigned long,
u32 handle, struct nlattr **,
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 1c122c7..7cf0a62 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -72,10 +72,6 @@ static unsigned long basic_get(struct tcf_proto *tp, u32 handle)
return l;
}
-static void basic_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int basic_init(struct tcf_proto *tp)
{
struct basic_head *head;
@@ -287,7 +283,6 @@ static struct tcf_proto_ops cls_basic_ops __read_mostly = {
.init = basic_init,
.destroy = basic_destroy,
.get = basic_get,
- .put = basic_put,
.change = basic_change,
.delete = basic_delete,
.walk = basic_walk,
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index d0de979..84c8219 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -151,10 +151,6 @@ static unsigned long cls_bpf_get(struct tcf_proto *tp, u32 handle)
return ret;
}
-static void cls_bpf_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
struct cls_bpf_prog *prog,
unsigned long base, struct nlattr **tb,
@@ -356,7 +352,6 @@ static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
.init = cls_bpf_init,
.destroy = cls_bpf_destroy,
.get = cls_bpf_get,
- .put = cls_bpf_put,
.change = cls_bpf_change,
.delete = cls_bpf_delete,
.walk = cls_bpf_walk,
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index dbee65e..741bfa7 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -67,10 +67,6 @@ static unsigned long cls_cgroup_get(struct tcf_proto *tp, u32 handle)
return 0UL;
}
-static void cls_cgroup_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int cls_cgroup_init(struct tcf_proto *tp)
{
return 0;
@@ -213,7 +209,6 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
.classify = cls_cgroup_classify,
.destroy = cls_cgroup_destroy,
.get = cls_cgroup_get,
- .put = cls_cgroup_put,
.delete = cls_cgroup_delete,
.walk = cls_cgroup_walk,
.dump = cls_cgroup_dump,
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 819c230..8e22718 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -581,10 +581,6 @@ static unsigned long flow_get(struct tcf_proto *tp, u32 handle)
return 0;
}
-static void flow_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int flow_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
struct sk_buff *skb, struct tcmsg *t)
{
@@ -671,7 +667,6 @@ static struct tcf_proto_ops cls_flow_ops __read_mostly = {
.change = flow_change,
.delete = flow_delete,
.get = flow_get,
- .put = flow_put,
.dump = flow_dump,
.walk = flow_walk,
.owner = THIS_MODULE,
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index dbfdfd1..23fda2a 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -111,10 +111,6 @@ static unsigned long fw_get(struct tcf_proto *tp, u32 handle)
return 0;
}
-static void fw_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int fw_init(struct tcf_proto *tp)
{
return 0;
@@ -411,7 +407,6 @@ static struct tcf_proto_ops cls_fw_ops __read_mostly = {
.init = fw_init,
.destroy = fw_destroy,
.get = fw_get,
- .put = fw_put,
.change = fw_change,
.delete = fw_delete,
.walk = fw_walk,
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 109a329..098a273 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -256,10 +256,6 @@ static unsigned long route4_get(struct tcf_proto *tp, u32 handle)
return 0;
}
-static void route4_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int route4_init(struct tcf_proto *tp)
{
return 0;
@@ -649,7 +645,6 @@ static struct tcf_proto_ops cls_route4_ops __read_mostly = {
.init = route4_init,
.destroy = route4_destroy,
.get = route4_get,
- .put = route4_put,
.change = route4_change,
.delete = route4_delete,
.walk = route4_walk,
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 6bb55f2..b7af362 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -271,10 +271,6 @@ static unsigned long rsvp_get(struct tcf_proto *tp, u32 handle)
return 0;
}
-static void rsvp_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static int rsvp_init(struct tcf_proto *tp)
{
struct rsvp_head *data;
@@ -708,7 +704,6 @@ static struct tcf_proto_ops RSVP_OPS __read_mostly = {
.init = rsvp_init,
.destroy = rsvp_destroy,
.get = rsvp_get,
- .put = rsvp_put,
.change = rsvp_change,
.delete = rsvp_delete,
.walk = rsvp_walk,
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 30f10fb..0d9d891 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -116,13 +116,6 @@ static unsigned long tcindex_get(struct tcf_proto *tp, u32 handle)
return r && tcindex_filter_is_set(r) ? (unsigned long) r : 0UL;
}
-
-static void tcindex_put(struct tcf_proto *tp, unsigned long f)
-{
- pr_debug("tcindex_put(tp %p,f 0x%lx)\n", tp, f);
-}
-
-
static int tcindex_init(struct tcf_proto *tp)
{
struct tcindex_data *p;
@@ -560,7 +553,6 @@ static struct tcf_proto_ops cls_tcindex_ops __read_mostly = {
.init = tcindex_init,
.destroy = tcindex_destroy,
.get = tcindex_get,
- .put = tcindex_put,
.change = tcindex_change,
.delete = tcindex_delete,
.walk = tcindex_walk,
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 0472909..09487af 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -299,10 +299,6 @@ static unsigned long u32_get(struct tcf_proto *tp, u32 handle)
return (unsigned long)u32_lookup_key(ht, handle);
}
-static void u32_put(struct tcf_proto *tp, unsigned long f)
-{
-}
-
static u32 gen_new_htid(struct tc_u_common *tp_c)
{
int i = 0x800;
@@ -1021,7 +1017,6 @@ static struct tcf_proto_ops cls_u32_ops __read_mostly = {
.init = u32_init,
.destroy = u32_destroy,
.get = u32_get,
- .put = u32_put,
.change = u32_change,
.delete = u32_delete,
.walk = u32_walk,
--
1.9.3
^ permalink raw reply related
* Re: [patch net-next v2 1/2] rocker: introduce be put/get variants and use it when appropriate
From: Scott Feldman @ 2014-12-04 20:33 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Netdev, David S. Miller
In-Reply-To: <1417612494-3788-1-git-send-email-jiri@resnulli.us>
Signed-off-by: Scott Feldman <sfeldma@gmail.com>
On Wed, Dec 3, 2014 at 5:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> This kills the sparse warnings.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> v1->v2:
> -no change
> ---
> drivers/net/ethernet/rocker/rocker.c | 79 ++++++++++++++++++++++--------------
> 1 file changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index fded127..4b060fb 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -648,6 +648,11 @@ static u16 rocker_tlv_get_u16(const struct rocker_tlv *tlv)
> return *(u16 *) rocker_tlv_data(tlv);
> }
>
> +static __be16 rocker_tlv_get_be16(const struct rocker_tlv *tlv)
> +{
> + return *(__be16 *) rocker_tlv_data(tlv);
> +}
> +
> static u32 rocker_tlv_get_u32(const struct rocker_tlv *tlv)
> {
> return *(u32 *) rocker_tlv_data(tlv);
> @@ -726,12 +731,24 @@ static int rocker_tlv_put_u16(struct rocker_desc_info *desc_info,
> return rocker_tlv_put(desc_info, attrtype, sizeof(u16), &value);
> }
>
> +static int rocker_tlv_put_be16(struct rocker_desc_info *desc_info,
> + int attrtype, __be16 value)
> +{
> + return rocker_tlv_put(desc_info, attrtype, sizeof(__be16), &value);
> +}
> +
> static int rocker_tlv_put_u32(struct rocker_desc_info *desc_info,
> int attrtype, u32 value)
> {
> return rocker_tlv_put(desc_info, attrtype, sizeof(u32), &value);
> }
>
> +static int rocker_tlv_put_be32(struct rocker_desc_info *desc_info,
> + int attrtype, __be32 value)
> +{
> + return rocker_tlv_put(desc_info, attrtype, sizeof(__be32), &value);
> +}
> +
> static int rocker_tlv_put_u64(struct rocker_desc_info *desc_info,
> int attrtype, u64 value)
> {
> @@ -1343,7 +1360,7 @@ static int rocker_event_mac_vlan_seen(struct rocker *rocker,
> port_number =
> rocker_tlv_get_u32(attrs[ROCKER_TLV_EVENT_MAC_VLAN_LPORT]) - 1;
> addr = rocker_tlv_data(attrs[ROCKER_TLV_EVENT_MAC_VLAN_MAC]);
> - vlan_id = rocker_tlv_get_u16(attrs[ROCKER_TLV_EVENT_MAC_VLAN_VLAN_ID]);
> + vlan_id = rocker_tlv_get_be16(attrs[ROCKER_TLV_EVENT_MAC_VLAN_VLAN_ID]);
>
> if (port_number >= rocker->port_count)
> return -EINVAL;
> @@ -1717,18 +1734,18 @@ static int rocker_cmd_flow_tbl_add_vlan(struct rocker_desc_info *desc_info,
> if (rocker_tlv_put_u32(desc_info, ROCKER_TLV_OF_DPA_IN_LPORT,
> entry->key.vlan.in_lport))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->key.vlan.vlan_id))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->key.vlan.vlan_id))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> - entry->key.vlan.vlan_id_mask))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> + entry->key.vlan.vlan_id_mask))
> return -EMSGSIZE;
> if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_GOTO_TABLE_ID,
> entry->key.vlan.goto_tbl))
> return -EMSGSIZE;
> if (entry->key.vlan.untagged &&
> - rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_NEW_VLAN_ID,
> - entry->key.vlan.new_vlan_id))
> + rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_NEW_VLAN_ID,
> + entry->key.vlan.new_vlan_id))
> return -EMSGSIZE;
>
> return 0;
> @@ -1743,8 +1760,8 @@ static int rocker_cmd_flow_tbl_add_term_mac(struct rocker_desc_info *desc_info,
> if (rocker_tlv_put_u32(desc_info, ROCKER_TLV_OF_DPA_IN_LPORT_MASK,
> entry->key.term_mac.in_lport_mask))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> - entry->key.term_mac.eth_type))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> + entry->key.term_mac.eth_type))
> return -EMSGSIZE;
> if (rocker_tlv_put(desc_info, ROCKER_TLV_OF_DPA_DST_MAC,
> ETH_ALEN, entry->key.term_mac.eth_dst))
> @@ -1752,11 +1769,11 @@ static int rocker_cmd_flow_tbl_add_term_mac(struct rocker_desc_info *desc_info,
> if (rocker_tlv_put(desc_info, ROCKER_TLV_OF_DPA_DST_MAC_MASK,
> ETH_ALEN, entry->key.term_mac.eth_dst_mask))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->key.term_mac.vlan_id))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->key.term_mac.vlan_id))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> - entry->key.term_mac.vlan_id_mask))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> + entry->key.term_mac.vlan_id_mask))
> return -EMSGSIZE;
> if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_GOTO_TABLE_ID,
> entry->key.term_mac.goto_tbl))
> @@ -1773,14 +1790,14 @@ static int
> rocker_cmd_flow_tbl_add_ucast_routing(struct rocker_desc_info *desc_info,
> struct rocker_flow_tbl_entry *entry)
> {
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> - entry->key.ucast_routing.eth_type))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> + entry->key.ucast_routing.eth_type))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u32(desc_info, ROCKER_TLV_OF_DPA_DST_IP,
> - entry->key.ucast_routing.dst4))
> + if (rocker_tlv_put_be32(desc_info, ROCKER_TLV_OF_DPA_DST_IP,
> + entry->key.ucast_routing.dst4))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u32(desc_info, ROCKER_TLV_OF_DPA_DST_IP_MASK,
> - entry->key.ucast_routing.dst4_mask))
> + if (rocker_tlv_put_be32(desc_info, ROCKER_TLV_OF_DPA_DST_IP_MASK,
> + entry->key.ucast_routing.dst4_mask))
> return -EMSGSIZE;
> if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_GOTO_TABLE_ID,
> entry->key.ucast_routing.goto_tbl))
> @@ -1804,8 +1821,8 @@ static int rocker_cmd_flow_tbl_add_bridge(struct rocker_desc_info *desc_info,
> ETH_ALEN, entry->key.bridge.eth_dst_mask))
> return -EMSGSIZE;
> if (entry->key.bridge.vlan_id &&
> - rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->key.bridge.vlan_id))
> + rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->key.bridge.vlan_id))
> return -EMSGSIZE;
> if (entry->key.bridge.tunnel_id &&
> rocker_tlv_put_u32(desc_info, ROCKER_TLV_OF_DPA_TUNNEL_ID,
> @@ -1846,14 +1863,14 @@ static int rocker_cmd_flow_tbl_add_acl(struct rocker_desc_info *desc_info,
> if (rocker_tlv_put(desc_info, ROCKER_TLV_OF_DPA_DST_MAC_MASK,
> ETH_ALEN, entry->key.acl.eth_dst_mask))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> - entry->key.acl.eth_type))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_ETHERTYPE,
> + entry->key.acl.eth_type))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->key.acl.vlan_id))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->key.acl.vlan_id))
> return -EMSGSIZE;
> - if (rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> - entry->key.acl.vlan_id_mask))
> + if (rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID_MASK,
> + entry->key.acl.vlan_id_mask))
> return -EMSGSIZE;
>
> switch (ntohs(entry->key.acl.eth_type)) {
> @@ -2002,8 +2019,8 @@ rocker_cmd_group_tbl_add_l2_rewrite(struct rocker_desc_info *desc_info,
> ETH_ALEN, entry->l2_rewrite.eth_dst))
> return -EMSGSIZE;
> if (entry->l2_rewrite.vlan_id &&
> - rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->l2_rewrite.vlan_id))
> + rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->l2_rewrite.vlan_id))
> return -EMSGSIZE;
>
> return 0;
> @@ -2048,8 +2065,8 @@ rocker_cmd_group_tbl_add_l3_unicast(struct rocker_desc_info *desc_info,
> ETH_ALEN, entry->l3_unicast.eth_dst))
> return -EMSGSIZE;
> if (entry->l3_unicast.vlan_id &&
> - rocker_tlv_put_u16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> - entry->l3_unicast.vlan_id))
> + rocker_tlv_put_be16(desc_info, ROCKER_TLV_OF_DPA_VLAN_ID,
> + entry->l3_unicast.vlan_id))
> return -EMSGSIZE;
> if (rocker_tlv_put_u8(desc_info, ROCKER_TLV_OF_DPA_TTL_CHECK,
> entry->l3_unicast.ttl_check))
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch net-next v2 2/2] rocker: fix eth_type type in struct rocker_ctrl
From: Scott Feldman @ 2014-12-04 20:33 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Netdev, David S. Miller
In-Reply-To: <1417612494-3788-2-git-send-email-jiri@resnulli.us>
Signed-off-by: Scott Feldman <sfeldma@gmail.com>
On Wed, Dec 3, 2014 at 5:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> v1->v2:
> -fixed typo in subject pointed out by Sergei
> ---
> drivers/net/ethernet/rocker/rocker.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c b/drivers/net/ethernet/rocker/rocker.c
> index 4b060fb..5536435 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -2753,7 +2753,7 @@ static int rocker_port_vlan_l2_groups(struct rocker_port *rocker_port,
> static struct rocker_ctrl {
> const u8 *eth_dst;
> const u8 *eth_dst_mask;
> - u16 eth_type;
> + __be16 eth_type;
> bool acl;
> bool bridge;
> bool term;
> --
> 1.9.3
>
^ permalink raw reply
* Re: [patch iproute2 1/6] iproute2: ipa: show switch id
From: Jiri Pirko @ 2014-12-04 20:27 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev, davem, nhorman, andy, tgraf, dborkman, ogerlitz, jesse,
pshelar, azhou, ben, stephen, jeffrey.t.kirsher, vyasevic,
xiyou.wangcong, john.r.fastabend, edumazet, jhs, sfeldma,
f.fainelli, roopa, linville, jasowang, nicolas.dichtel,
ryazanov.s.a, buytenh, aviadr, nbd, alexei.starovoitov,
Neil.Jerram, ronye, simon.horman, alexander.h.duyck, john.ronciak,
mleitner, shrijeet, gospo, bcrl, hemal
In-Reply-To: <87h9xbxjrd.fsf@x220.int.ebiederm.org>
Thu, Dec 04, 2014 at 09:06:14PM CET, ebiederm@xmission.com wrote:
>ebiederm@xmission.com (Eric W. Biederman) writes:
>
>> Jiri Pirko <jiri@resnulli.us> writes:
>>
>>>>So this id needs to be globally unique?
>>>
>>> No. It is enough to be unique within a single system. It serves for no
>>> more than to find out 2 ids are same or not, no other info value.
>>>
>>> So when the drivers uses sane ids (like mac for example, or in case of
>>> rocker an id which is passed by qemu command line), the chances of
>>> collision are very very close to none (never say never).
>
>Thinking about what you said a little more.
>
>Two different sources of persistent numbers picking numbers by
>completely different algorithms can give you no assurance that you don't
>produce conflicts.
>
>The switch id as desisgned can not work.
>
>There are expected to be between 2**36 to 2**40 devices in this world.
>Your first switch id is a 64it number. At the very best by the birthday
>pardox predicts there will be a conflict ever 2**32 devices or between
>2**4 and 2**8 devices in the world with conflicts. If the ids are not
>randomly distributed (which they won't be) things could easily be much
>much worse.
>
>That is just good enough the code could get out there and run for years
>before you have the nightmare of having to fix all of userspace. That
>is a nightmare no one needs.
>
>So please remove this broken code, and this broken concept from the
>kernel and go back to the drawing board.
In that case the phys port id is broken in the same way. Let's rather
think about how to avoid conflicts for both. Given the fact the
conflicts should be avoided only on a single baremetal, that should be
doable (for (bad) example using driver name mixed with driver created id).
^ permalink raw reply
* [RFC][PATCH 13/13] copy_from_iter_nocache()
From: Al Viro @ 2014-12-04 20:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel, netdev
In-Reply-To: <20141204202011.GO29748@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
BTW, do we want memcpy_nocache()?
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/uio.h | 1 +
mm/iov_iter.c | 21 +++++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index c567655..bd8569a 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -83,6 +83,7 @@ size_t copy_page_from_iter(struct page *page, size_t offset, size_t bytes,
struct iov_iter *i);
size_t copy_to_iter(void *addr, size_t bytes, struct iov_iter *i);
size_t copy_from_iter(void *addr, size_t bytes, struct iov_iter *i);
+size_t copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i);
size_t iov_iter_zero(size_t bytes, struct iov_iter *);
unsigned long iov_iter_alignment(const struct iov_iter *i);
void iov_iter_init(struct iov_iter *i, int direction, const struct iovec *iov,
diff --git a/mm/iov_iter.c b/mm/iov_iter.c
index 7c04051..e0605c1 100644
--- a/mm/iov_iter.c
+++ b/mm/iov_iter.c
@@ -399,6 +399,27 @@ size_t copy_from_iter(void *addr, size_t bytes, struct iov_iter *i)
}
EXPORT_SYMBOL(copy_from_iter);
+size_t copy_from_iter_nocache(void *addr, size_t bytes, struct iov_iter *i)
+{
+ char *to = addr;
+ if (unlikely(bytes > i->count))
+ bytes = i->count;
+
+ if (unlikely(!bytes))
+ return 0;
+
+ iterate_and_advance(i, bytes, v,
+ __copy_from_user_nocache((to += v.iov_len) - v.iov_len,
+ v.iov_base, v.iov_len),
+ memcpy_from_page((to += v.bv_len) - v.bv_len, v.bv_page,
+ v.bv_offset, v.bv_len),
+ memcpy((to += v.iov_len) - v.iov_len, v.iov_base, v.iov_len)
+ )
+
+ return bytes;
+}
+EXPORT_SYMBOL(copy_from_iter_nocache);
+
size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
struct iov_iter *i)
{
--
2.1.3
^ permalink raw reply related
* [RFC][PATCH 12/13] new helper: iov_iter_kvec()
From: Al Viro @ 2014-12-04 20:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-fsdevel, netdev
In-Reply-To: <20141204202011.GO29748@ZenIV.linux.org.uk>
From: Al Viro <viro@zeniv.linux.org.uk>
initialization of kvec-backed iov_iter
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
include/linux/uio.h | 2 ++
mm/iov_iter.c | 13 +++++++++++++
2 files changed, 15 insertions(+)
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 28ed2d9..c567655 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -87,6 +87,8 @@ size_t iov_iter_zero(size_t bytes, struct iov_iter *);
unsigned long iov_iter_alignment(const struct iov_iter *i);
void iov_iter_init(struct iov_iter *i, int direction, const struct iovec *iov,
unsigned long nr_segs, size_t count);
+void iov_iter_kvec(struct iov_iter *i, int direction, const struct kvec *iov,
+ unsigned long nr_segs, size_t count);
ssize_t iov_iter_get_pages(struct iov_iter *i, struct page **pages,
size_t maxsize, unsigned maxpages, size_t *start);
ssize_t iov_iter_get_pages_alloc(struct iov_iter *i, struct page ***pages,
diff --git a/mm/iov_iter.c b/mm/iov_iter.c
index 5a3b4a8..7c04051 100644
--- a/mm/iov_iter.c
+++ b/mm/iov_iter.c
@@ -479,6 +479,19 @@ size_t iov_iter_single_seg_count(const struct iov_iter *i)
}
EXPORT_SYMBOL(iov_iter_single_seg_count);
+void iov_iter_kvec(struct iov_iter *i, int direction,
+ const struct kvec *iov, unsigned long nr_segs,
+ size_t count)
+{
+ BUG_ON(!(direction & ITER_KVEC));
+ i->type = direction;
+ i->kvec = (struct kvec *)iov;
+ i->nr_segs = nr_segs;
+ i->iov_offset = 0;
+ i->count = count;
+}
+EXPORT_SYMBOL(iov_iter_kvec);
+
unsigned long iov_iter_alignment(const struct iov_iter *i)
{
unsigned long res = 0;
--
2.1.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox