* Re: [PATCH] net/phy: micrel: Disable asymmetric pause for KSZ9031
From: Mike Looijmans @ 2014-09-12 6:54 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: linux-kernel
In-Reply-To: <5411ECE9.8050600@gmail.com>
On 09/11/2014 08:41 PM, Florian Fainelli wrote:
> On 09/11/2014 06:45 AM, Mike Looijmans wrote:
>> Our KSZ9031 appears to suffer from the same hardware bug as described
>> for the KSZ9021 in commit 32fcafbcd1c9f6c7013016a22a5369b4acb93577,
>> you have to unplug the cable and plug it back to get it to work.
>>
>> Remove the SUPPORTED_Asym_Pause flag for the KSZ9031 to fix this.
>
> Can you resend this patch specifying the commit subject in parenthesis
> like this:
>
> commit 32fcafbcd1c9f6c7013016a22a5369b4acb93577 ("net/phy: micrel:
> Disable asymmetric pause for KSZ9021")
I'll amend the text body and resend the patch.
> since this is a bugfix, it should probably be targeting the 'net' tree,
> rather than the 'net-next' tree, though the patch applies to both cleanly.
I assume this does not require any action on my side? I actually based
this patch on a 3.14 kernel.
> Other than that:
>
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>> ---
>> drivers/net/phy/micrel.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
>> index 5a8993b..a932a35 100644
>> --- a/drivers/net/phy/micrel.c
>> +++ b/drivers/net/phy/micrel.c
>> @@ -425,8 +425,7 @@ static struct phy_driver ksphy_driver[] = {
>> .phy_id = PHY_ID_KSZ9031,
>> .phy_id_mask = 0x00fffff0,
>> .name = "Micrel KSZ9031 Gigabit PHY",
>> - .features = (PHY_GBIT_FEATURES | SUPPORTED_Pause
>> - | SUPPORTED_Asym_Pause),
>> + .features = (PHY_GBIT_FEATURES | SUPPORTED_Pause),
>> .flags = PHY_HAS_MAGICANEG | PHY_HAS_INTERRUPT,
>> .config_init = kszphy_config_init,
>> .config_aneg = genphy_config_aneg,
>>
>
--
Mike Looijmans
^ permalink raw reply
* Re: PTP_PEROUT_REQUEST and clock stepping
From: Richard Cochran @ 2014-09-12 6:33 UTC (permalink / raw)
To: Christian Riesch; +Cc: Daniel Glöckner, netdev@vger.kernel.org
In-Reply-To: <CABkLOboeo_+37rE84gPTEKnmXD+599fRtnBZSR465y9aZeOdrw@mail.gmail.com>
On Fri, Sep 12, 2014 at 08:24:49AM +0200, Christian Riesch wrote:
> On Thu, Sep 11, 2014 at 11:52 AM, Daniel Glöckner <dg@emlix.com> wrote:
> > Only a small fraction of people
> > using periodic outputs will want to schedule the start of the signal at
> > a specific time in the future and only a fraction of those will use a
> > .period value that is so small that they can't use a timer to configure
> > the periodic output within .period/2 before the desired start time.
>
> Ok, I got it now. So you are suggesting to change the behavior of
> PTP_PEROUT_REQUEST, aren't you? And break existing applications?
> Christian
I think he wants the periodic output to continue at the same phase and
frequency, even when you reset the clock time.
However, this is impossible to acheive because the hardware does not
support it.
Thanks,
Richard
^ permalink raw reply
* Re: PTP_PEROUT_REQUEST and clock stepping
From: Christian Riesch @ 2014-09-12 6:24 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Richard Cochran, netdev@vger.kernel.org
In-Reply-To: <20140911095245.GA5823@emlix.com>
On Thu, Sep 11, 2014 at 11:52 AM, Daniel Glöckner <dg@emlix.com> wrote:
> Only a small fraction of people
> using periodic outputs will want to schedule the start of the signal at
> a specific time in the future and only a fraction of those will use a
> .period value that is so small that they can't use a timer to configure
> the periodic output within .period/2 before the desired start time.
Ok, I got it now. So you are suggesting to change the behavior of
PTP_PEROUT_REQUEST, aren't you? And break existing applications?
Christian
^ permalink raw reply
* Re: PTP_PEROUT_REQUEST and clock stepping
From: Christian Riesch @ 2014-09-12 6:17 UTC (permalink / raw)
To: Daniel Glöckner; +Cc: Richard Cochran, netdev@vger.kernel.org
In-Reply-To: <20140911095245.GA5823@emlix.com>
Hi Daniel,
On Thu, Sep 11, 2014 at 11:52 AM, Daniel Glöckner <dg@emlix.com> wrote:
> On Wed, Sep 10, 2014 at 07:26:20PM +0200, Richard Cochran wrote:
>> On Wed, Sep 10, 2014 at 06:16:37PM +0200, Daniel Glöckner wrote:
>> > I was wondering how periodic output is supposed to behave if the clock
>> > is stepped from a time after ptp_perout_request.start to a time before
>> > ptp_perout_request.start.
>>
>> I would say the result is undefined.
>>
>> User space should first stop the periodic output, then reprogram the
>> clock, then restart the periodic output. Anything else makes no sense
>> at all.
>
> But then the PTP demon has to be responsible for configuring the periodic
> outputs as well.
I guess yes, but is that a problem?
> IMHO there is no use for a .start value > .period. It is good for defining
There is, for the small fraction of people you note below. But you are
talking about PTP_PEROUT_REQUEST here, right?
> the phase of the periodic output but is just annoying if you want to have
> the periodic output running all the time.
How about PTP_ENABLE_PPS? Is this the ptp clock feature you are looking for?
> Only a small fraction of people
> using periodic outputs will want to schedule the start of the signal at
> a specific time in the future and only a fraction of those will use a
> .period value that is so small that they can't use a timer to configure
> the periodic output within .period/2 before the desired start time.
What timer?
Regards,
Christian
^ permalink raw reply
* Re: [PATCH net-next v4 2/9] net: filter: keep original BPF program around
From: Daniel Borkmann @ 2014-09-12 6:09 UTC (permalink / raw)
To: Eric Dumazet; +Cc: davem, ast, netdev, Pavel Emelyanov
In-Reply-To: <1410492457.7106.72.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/12/2014 05:27 AM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> On Fri, 2014-03-28 at 18:58 +0100, Daniel Borkmann wrote:
>> In order to open up the possibility to internally transform a BPF program
>> into an alternative and possibly non-trivial reversible representation, we
>> need to keep the original BPF program around, so that it can be passed back
>> to user space w/o the need of a complex decoder.
>>
>> The reason for that use case resides in commit a8fc92778080 ("sk-filter:
>> Add ability to get socket filter program (v2)"), that is, the ability
>> to retrieve the currently attached BPF filter from a given socket used
>> mainly by the checkpoint-restore project, for example.
>>
>> Therefore, we add two helpers sk_{store,release}_orig_filter for taking
>> care of that. In the sk_unattached_filter_create() case, there's no such
>> possibility/requirement to retrieve a loaded BPF program. Therefore, we
>> can spare us the work in that case.
>>
>> This approach will simplify and slightly speed up both, sk_get_filter()
>> and sock_diag_put_filterinfo() handlers as we won't need to successively
>> decode filters anymore through sk_decode_filter(). As we still need
>> sk_decode_filter() later on, we're keeping it around.
>>
>> Joint work with Alexei Starovoitov.
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Cc: Pavel Emelyanov <xemul@parallels.com>
>> ---
>
> Note that this patch added a possible use after free.
>
> Following patch should be sent to stable trees only, as 3.17+
> incidentally fixed this with
> commit 278571baca2aecf5fb5cb5c8b002dbfa0a6c524c
> net: filter: simplify socket charging
>
> [PATCH] net: filter: fix possible use after free
>
> If kmemdup() fails, we free fp->orig_prog and return -ENOMEM
>
> sk_attach_filter()
> -> sk_filter_uncharge(sk, fp)
> -> sk_filter_release(fp)
> -> call_rcu(&fp->rcu, sk_filter_release_rcu)
> -> sk_filter_release_rcu()
> -> sk_release_orig_filter()
> fprog = fp->orig_prog; // not NULL, but points to freed memory
> kfree(fprog->filter); // use after free, potential corruption
> kfree(fprog); // double free or corruption
>
> Note: This was fixed in 3.17+ with commit 278571baca2a
> ("net: filter: simplify socket charging")
>
> Found by AddressSanitizer
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: a3ea269b8bcdb ("net: filter: keep original BPF program around")
Thanks Eric!
Acked-by: Daniel Borkmann <dborkman@redhat.com>
^ permalink raw reply
* Re: [RFC v2 3/6] kthread: warn on kill signal if not OOM
From: Tom Gundersen @ 2014-09-12 5:48 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: One Thousand Gnomes, Takashi Iwai, Kay Sievers, Sreekanth Reddy,
James Bottomley, Praveen Krishnamoorthy, hare,
Nagalakshmi Nandigama, Wu Zhangjin, Tetsuo Handa,
mpt-fusionlinux.pdl, Tim Gardner, Benjamin Poirier,
Santosh Rastapur, Casey Leedom, Hariprasad S, Pierre Fersing,
Arjan van de Ven, Abhijit Mahajan, systemd Mailing List
In-Reply-To: <CAB=NE6XmFaWMKuW3vJ7QMZ1BmmWrAPezTYYsqatX4mMNdMjpog@mail.gmail.com>
On Fri, Sep 12, 2014 at 12:26 AM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> On Thu, Sep 11, 2014 at 2:43 PM, Tom Gundersen <teg@jklm.no> wrote:
>> How about simply introducing a new flag to finit_module() to indicate
>> that the caller does not care about asynchronicity. We could then pass
>> this from udev, but existing scripts calling modprobe/insmod will not
>> be affected.
>
> Do you mean that you *do want asynchronicity*?
Precisely, udev would opt-in, but existing scripts etc would not.
>> But isn't finit_module() taking a long time a serious problem given
>> that it means no other module can be loaded in parallel?
>
> Indeed but having a desire to make the init() complete fast is
> different than the desire to have the combination of both init and
> probe fast synchronously.
I guess no one is arguing that probe should somehow be required to be
fast, but rather:
> If userspace wants init to be fast and let
> probe be async then userspace has no option but to deal with the fact
> that async probe will be async, and it should then use other methods
> to match any dependencies if its doing that itself.
Correct. And this therefore likely needs to be opt-in behaviour per
finit_module() invocation to avoid breaking old assumptions.
> For example
> networking should not kick off after a network driver is loaded but
> rather one the device creeps up on udev. We should be good with
> networking dealing with this correctly today but not sure about other
> subsystems. depmod should be able to load the required modules in
> order and if bus drivers work right then probe of the remnant devices
> should happen asynchronously. The one case I can think of that is a
> bit different is modules-load.d things but those *do not rely on the
> timeout*, but are loaded prior to a service requirement. Note though
> that if those modules had probe and they then run async'd then systemd
> service would probably need to consider that the requirements may not
> be there until later. If this is not carefully considered that could
> introduce regression to users of modules-load.d when async probe is
> fully deployed. The same applies to systemd making assumptions of kmod
> loading a module and a dependency being complete as probe would have
> run it before.
Yeah, these all needs to be considered when deciding whether or not to
enable async in each specific case.
> I believe one concern here lies in on whether or not userspace
> is properly equipped to deal with the requirements on module loading
> doing async probing and that possibly failing. Perhaps systemd might
> think all userspace is ready for that but are we sure that's the case?
There almost certainly are custom things out there relying on the
synchronous behaviour, but if we make it opt-in we should not have a
problem.
Cheers,
Tom
^ permalink raw reply
* [PATCH net-next v2] net: dsa: change tag_protocol to an enum
From: Florian Fainelli @ 2014-09-12 4:18 UTC (permalink / raw)
To: netdev; +Cc: davem, kernel, alexander.duyck, Florian Fainelli
Now that we introduced an additional multiplexing/demultiplexing layer
with commit 3e8a72d1dae37 ("net: dsa: reduce number of protocol hooks")
that lives within the DSA code, we no longer need to have a given switch
driver tag_protocol be an actual ethertype value, instead, we can
replace it with an enum: dsa_tag_protocol.
Do this replacement in the drivers, which allows us to get rid of the
cpu_to_be16()/htons() dance, and remove ETH_P_BRCMTAG since we do not
need it anymore.
Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- removed now obsolete comment about the former ETH_P_BRCMTAG we did
introduce earlier
drivers/net/dsa/bcm_sf2.c | 2 +-
drivers/net/dsa/mv88e6060.c | 2 +-
drivers/net/dsa/mv88e6123_61_65.c | 4 ++--
drivers/net/dsa/mv88e6131.c | 2 +-
include/net/dsa.h | 17 ++++++++++-------
net/dsa/slave.c | 8 ++++----
net/dsa/tag_brcm.c | 1 -
7 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index bb7cb8e..e9918c7 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -592,7 +592,7 @@ static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
}
static struct dsa_switch_driver bcm_sf2_switch_driver = {
- .tag_protocol = htons(ETH_P_BRCMTAG),
+ .tag_protocol = DSA_TAG_PROTO_BRCM,
.priv_size = sizeof(struct bcm_sf2_priv),
.probe = bcm_sf2_sw_probe,
.setup = bcm_sf2_sw_setup,
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 7a54ec0..d8037c1 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -258,7 +258,7 @@ static void mv88e6060_poll_link(struct dsa_switch *ds)
}
static struct dsa_switch_driver mv88e6060_switch_driver = {
- .tag_protocol = htons(ETH_P_TRAILER),
+ .tag_protocol = DSA_TAG_PROTO_TRAILER,
.probe = mv88e6060_probe,
.setup = mv88e6060_setup,
.set_addr = mv88e6060_set_addr,
diff --git a/drivers/net/dsa/mv88e6123_61_65.c b/drivers/net/dsa/mv88e6123_61_65.c
index 69c4251..975774f 100644
--- a/drivers/net/dsa/mv88e6123_61_65.c
+++ b/drivers/net/dsa/mv88e6123_61_65.c
@@ -207,7 +207,7 @@ static int mv88e6123_61_65_setup_port(struct dsa_switch *ds, int p)
*/
val = 0x0433;
if (dsa_is_cpu_port(ds, p)) {
- if (ds->dst->tag_protocol == htons(ETH_P_EDSA))
+ if (ds->dst->tag_protocol == DSA_TAG_PROTO_EDSA)
val |= 0x3300;
else
val |= 0x0100;
@@ -391,7 +391,7 @@ static int mv88e6123_61_65_get_sset_count(struct dsa_switch *ds)
}
struct dsa_switch_driver mv88e6123_61_65_switch_driver = {
- .tag_protocol = cpu_to_be16(ETH_P_EDSA),
+ .tag_protocol = DSA_TAG_PROTO_EDSA,
.priv_size = sizeof(struct mv88e6xxx_priv_state),
.probe = mv88e6123_61_65_probe,
.setup = mv88e6123_61_65_setup,
diff --git a/drivers/net/dsa/mv88e6131.c b/drivers/net/dsa/mv88e6131.c
index 953bc6a..35541f2 100644
--- a/drivers/net/dsa/mv88e6131.c
+++ b/drivers/net/dsa/mv88e6131.c
@@ -379,7 +379,7 @@ static int mv88e6131_get_sset_count(struct dsa_switch *ds)
}
struct dsa_switch_driver mv88e6131_switch_driver = {
- .tag_protocol = cpu_to_be16(ETH_P_DSA),
+ .tag_protocol = DSA_TAG_PROTO_DSA,
.priv_size = sizeof(struct mv88e6xxx_priv_state),
.probe = mv88e6131_probe,
.setup = mv88e6131_setup,
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 9771292..8a8a5d9 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -19,10 +19,13 @@
#include <linux/phy.h>
#include <linux/phy_fixed.h>
-/* Not an official ethertype value, used only internally for DSA
- * demultiplexing
- */
-#define ETH_P_BRCMTAG (ETH_P_XDSA + 1)
+enum dsa_tag_protocol {
+ DSA_TAG_PROTO_NONE = 0,
+ DSA_TAG_PROTO_DSA,
+ DSA_TAG_PROTO_TRAILER,
+ DSA_TAG_PROTO_EDSA,
+ DSA_TAG_PROTO_BRCM,
+};
#define DSA_MAX_SWITCHES 4
#define DSA_MAX_PORTS 12
@@ -89,7 +92,7 @@ struct dsa_switch_tree {
*/
struct net_device *master_netdev;
const struct dsa_device_ops *ops;
- __be16 tag_protocol;
+ enum dsa_tag_protocol tag_protocol;
/*
* The switch and port to which the CPU is attached.
@@ -166,7 +169,7 @@ static inline u8 dsa_upstream_port(struct dsa_switch *ds)
struct dsa_switch_driver {
struct list_head list;
- __be16 tag_protocol;
+ enum dsa_tag_protocol tag_protocol;
int priv_size;
/*
@@ -215,7 +218,7 @@ static inline void *ds_to_priv(struct dsa_switch *ds)
static inline bool dsa_uses_tagged_protocol(struct dsa_switch_tree *dst)
{
- return dst->tag_protocol != 0;
+ return dst->tag_protocol != DSA_TAG_PROTO_NONE;
}
#endif
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7333a4a..809eeb1 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -437,22 +437,22 @@ dsa_slave_create(struct dsa_switch *ds, struct device *parent,
switch (ds->dst->tag_protocol) {
#ifdef CONFIG_NET_DSA_TAG_DSA
- case htons(ETH_P_DSA):
+ case DSA_TAG_PROTO_DSA:
ds->dst->ops = &dsa_netdev_ops;
break;
#endif
#ifdef CONFIG_NET_DSA_TAG_EDSA
- case htons(ETH_P_EDSA):
+ case DSA_TAG_PROTO_EDSA:
ds->dst->ops = &edsa_netdev_ops;
break;
#endif
#ifdef CONFIG_NET_DSA_TAG_TRAILER
- case htons(ETH_P_TRAILER):
+ case DSA_TAG_PROTO_TRAILER:
ds->dst->ops = &trailer_netdev_ops;
break;
#endif
#ifdef CONFIG_NET_DSA_TAG_BRCM
- case htons(ETH_P_BRCMTAG):
+ case DSA_TAG_PROTO_BRCM:
ds->dst->ops = &brcm_netdev_ops;
break;
#endif
diff --git a/net/dsa/tag_brcm.c b/net/dsa/tag_brcm.c
index e0b759e..8fbc21c 100644
--- a/net/dsa/tag_brcm.c
+++ b/net/dsa/tag_brcm.c
@@ -91,7 +91,6 @@ static netdev_tx_t brcm_tag_xmit(struct sk_buff *skb, struct net_device *dev)
/* Queue the SKB for transmission on the parent interface, but
* do not modify its EtherType
*/
- skb->protocol = htons(ETH_P_BRCMTAG);
skb->dev = p->parent->dst->master_netdev;
dev_queue_xmit(skb);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 net-next] net: filter: export pkt_type_offset() helper
From: Alexei Starovoitov @ 2014-09-12 4:01 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, David Laight, Denis Kirjanov, Daniel Borkmann,
Eric Dumazet, Denis Kirjanov, netdev@vger.kernel.org,
Markos Chandras, Martin Schwidefsky
In-Reply-To: <1409870759.2310959.163817265.10353D86@webmail.messagingengine.com>
On Thu, Sep 4, 2014 at 3:45 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> >
>> > I think the latest proposals looks good?
>>
>> to me: yes
>> and I think your latest half-patch with:
>> + __u8 __pkt_type_offset[0];
>> also looks good.
>>
>> Are you going to take it over from Denis here?
>
> I don't know. Denis, do you want to incorperate my changes or should I
> take over here?
Hannes, Denis,
anyone of you going to continue on that?
imo it's a good cleanup.
^ permalink raw reply
* Re: [PATCH net-next v4 2/9] net: filter: keep original BPF program around
From: Alexei Starovoitov @ 2014-09-12 3:51 UTC (permalink / raw)
To: Eric Dumazet
Cc: Daniel Borkmann, David S. Miller, Network Development,
Pavel Emelyanov
In-Reply-To: <1410492457.7106.72.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Sep 11, 2014 at 8:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> On Fri, 2014-03-28 at 18:58 +0100, Daniel Borkmann wrote:
>> In order to open up the possibility to internally transform a BPF program
>> into an alternative and possibly non-trivial reversible representation, we
>> need to keep the original BPF program around, so that it can be passed back
>> to user space w/o the need of a complex decoder.
>>
>> The reason for that use case resides in commit a8fc92778080 ("sk-filter:
>> Add ability to get socket filter program (v2)"), that is, the ability
>> to retrieve the currently attached BPF filter from a given socket used
>> mainly by the checkpoint-restore project, for example.
>>
>> Therefore, we add two helpers sk_{store,release}_orig_filter for taking
>> care of that. In the sk_unattached_filter_create() case, there's no such
>> possibility/requirement to retrieve a loaded BPF program. Therefore, we
>> can spare us the work in that case.
>>
>> This approach will simplify and slightly speed up both, sk_get_filter()
>> and sock_diag_put_filterinfo() handlers as we won't need to successively
>> decode filters anymore through sk_decode_filter(). As we still need
>> sk_decode_filter() later on, we're keeping it around.
>>
>> Joint work with Alexei Starovoitov.
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> Cc: Pavel Emelyanov <xemul@parallels.com>
>> ---
>
> Note that this patch added a possible use after free.
>
> Following patch should be sent to stable trees only, as 3.17+
> incidentally fixed this with
> commit 278571baca2aecf5fb5cb5c8b002dbfa0a6c524c
> net: filter: simplify socket charging
>
> [PATCH] net: filter: fix possible use after free
>
> If kmemdup() fails, we free fp->orig_prog and return -ENOMEM
>
> sk_attach_filter()
> -> sk_filter_uncharge(sk, fp)
> -> sk_filter_release(fp)
> -> call_rcu(&fp->rcu, sk_filter_release_rcu)
> -> sk_filter_release_rcu()
> -> sk_release_orig_filter()
> fprog = fp->orig_prog; // not NULL, but points to freed memory
> kfree(fprog->filter); // use after free, potential corruption
> kfree(fprog); // double free or corruption
>
> Note: This was fixed in 3.17+ with commit 278571baca2a
> ("net: filter: simplify socket charging")
>
> Found by AddressSanitizer
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: a3ea269b8bcdb ("net: filter: keep original BPF program around")
Wow. that's a tricky one. Thanks!
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
Indeed 3.17 is ok, but the fix is small enough, so wouldn't
hurt to apply it everywhere. Just for consistency?
> ---
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 1dbf6462f766..3139f966a178 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -1318,6 +1318,7 @@ static int sk_store_orig_filter(struct sk_filter *fp,
> fkprog->filter = kmemdup(fp->insns, fsize, GFP_KERNEL);
> if (!fkprog->filter) {
> kfree(fp->orig_prog);
> + fp->orig_prog = NULL;
> return -ENOMEM;
> }
>
>
>
^ permalink raw reply
* [PATCH net 2/2] r8169: fix setting rx vlan
From: Hayes Wang @ 2014-09-12 3:35 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-38-Taiwan-albertk@realtek.com>
The setting should depend on the new features not the current one.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/ethernet/realtek/r8169.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index f3ce284..7a7860a 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -1796,7 +1796,7 @@ static void __rtl8169_set_features(struct net_device *dev,
else
tp->cp_cmd &= ~RxChkSum;
- if (dev->features & NETIF_F_HW_VLAN_CTAG_RX)
+ if (features & NETIF_F_HW_VLAN_CTAG_RX)
tp->cp_cmd |= RxVlan;
else
tp->cp_cmd &= ~RxVlan;
--
1.9.3
^ permalink raw reply related
* [PATCH net 1/2] r8169: fix the default setting of rx vlan
From: Hayes Wang @ 2014-09-12 3:35 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
In-Reply-To: <1394712342-15778-38-Taiwan-albertk@realtek.com>
If the parameter "features" of __rtl8169_set_features() is equal to
dev->features, the variable "changed" is alwayes 0, and nothing would
be changed.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/ethernet/realtek/r8169.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/realtek/r8169.c b/drivers/net/ethernet/realtek/r8169.c
index 91652e7..f3ce284 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -6707,7 +6707,12 @@ static int rtl_open(struct net_device *dev)
rtl8169_init_phy(dev, tp);
- __rtl8169_set_features(dev, dev->features);
+ if (dev->features & NETIF_F_HW_VLAN_CTAG_RX)
+ tp->cp_cmd |= RxVlan;
+ else
+ tp->cp_cmd &= ~RxVlan;
+
+ RTL_W16(CPlusCmd, tp->cp_cmd);
rtl_pll_power_up(tp);
--
1.9.3
^ permalink raw reply related
* [PATCH net 0/2] r8169: fix rx vlan
From: Hayes Wang @ 2014-09-12 3:35 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, Hayes Wang
There are two issues for hw rx vlan. The patches are
used to fix them.
Hayes Wang (2):
r8169: fix the default setting of rx vlan
r8169: fix setting rx vlan
drivers/net/ethernet/realtek/r8169.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--
1.9.3
^ permalink raw reply
* Re: [PATCH net-next v4 2/9] net: filter: keep original BPF program around
From: Eric Dumazet @ 2014-09-12 3:27 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, ast, netdev, Pavel Emelyanov
In-Reply-To: <1396029506-16776-3-git-send-email-dborkman@redhat.com>
From: Eric Dumazet <edumazet@google.com>
On Fri, 2014-03-28 at 18:58 +0100, Daniel Borkmann wrote:
> In order to open up the possibility to internally transform a BPF program
> into an alternative and possibly non-trivial reversible representation, we
> need to keep the original BPF program around, so that it can be passed back
> to user space w/o the need of a complex decoder.
>
> The reason for that use case resides in commit a8fc92778080 ("sk-filter:
> Add ability to get socket filter program (v2)"), that is, the ability
> to retrieve the currently attached BPF filter from a given socket used
> mainly by the checkpoint-restore project, for example.
>
> Therefore, we add two helpers sk_{store,release}_orig_filter for taking
> care of that. In the sk_unattached_filter_create() case, there's no such
> possibility/requirement to retrieve a loaded BPF program. Therefore, we
> can spare us the work in that case.
>
> This approach will simplify and slightly speed up both, sk_get_filter()
> and sock_diag_put_filterinfo() handlers as we won't need to successively
> decode filters anymore through sk_decode_filter(). As we still need
> sk_decode_filter() later on, we're keeping it around.
>
> Joint work with Alexei Starovoitov.
>
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> ---
Note that this patch added a possible use after free.
Following patch should be sent to stable trees only, as 3.17+
incidentally fixed this with
commit 278571baca2aecf5fb5cb5c8b002dbfa0a6c524c
net: filter: simplify socket charging
[PATCH] net: filter: fix possible use after free
If kmemdup() fails, we free fp->orig_prog and return -ENOMEM
sk_attach_filter()
-> sk_filter_uncharge(sk, fp)
-> sk_filter_release(fp)
-> call_rcu(&fp->rcu, sk_filter_release_rcu)
-> sk_filter_release_rcu()
-> sk_release_orig_filter()
fprog = fp->orig_prog; // not NULL, but points to freed memory
kfree(fprog->filter); // use after free, potential corruption
kfree(fprog); // double free or corruption
Note: This was fixed in 3.17+ with commit 278571baca2a
("net: filter: simplify socket charging")
Found by AddressSanitizer
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: a3ea269b8bcdb ("net: filter: keep original BPF program around")
---
diff --git a/net/core/filter.c b/net/core/filter.c
index 1dbf6462f766..3139f966a178 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1318,6 +1318,7 @@ static int sk_store_orig_filter(struct sk_filter *fp,
fkprog->filter = kmemdup(fp->insns, fsize, GFP_KERNEL);
if (!fkprog->filter) {
kfree(fp->orig_prog);
+ fp->orig_prog = NULL;
return -ENOMEM;
}
^ permalink raw reply related
* [PATCH net-next v2] r8152: support VLAN
From: Hayes Wang @ 2014-09-12 2:43 UTC (permalink / raw)
To: netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-36-Taiwan-albertk@realtek.com>
Support hw VLAN for tx and rx. And enable them by default.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 79 ++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 65 insertions(+), 14 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 2130c75..9403219 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -506,6 +506,7 @@ struct rx_desc {
#define IPF (1 << 23) /* IP checksum fail */
#define UDPF (1 << 22) /* UDP checksum fail */
#define TCPF (1 << 21) /* TCP checksum fail */
+#define RX_VLAN_TAG (1 << 16)
__le32 opts4;
__le32 opts5;
@@ -531,6 +532,7 @@ struct tx_desc {
#define MSS_MAX 0x7ffU
#define TCPHO_SHIFT 17
#define TCPHO_MAX 0x7ffU
+#define TX_VLAN_TAG (1 << 16)
};
struct r8152;
@@ -1423,6 +1425,25 @@ static int msdn_giant_send_check(struct sk_buff *skb)
return ret;
}
+static inline void rtl_tx_vlan_tag(struct tx_desc *desc, struct sk_buff *skb)
+{
+ if (vlan_tx_tag_present(skb)) {
+ u32 opts2;
+
+ opts2 = TX_VLAN_TAG | swab16(vlan_tx_tag_get(skb));
+ desc->opts2 |= cpu_to_le32(opts2);
+ }
+}
+
+static inline void rtl_rx_vlan_tag(struct rx_desc *desc, struct sk_buff *skb)
+{
+ u32 opts2 = le32_to_cpu(desc->opts2);
+
+ if (opts2 & RX_VLAN_TAG)
+ __vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q),
+ swab16(opts2 & 0xffff));
+}
+
static int r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc,
struct sk_buff *skb, u32 len, u32 transport_offset)
{
@@ -1550,6 +1571,8 @@ static int r8152_tx_agg_fill(struct r8152 *tp, struct tx_agg *agg)
continue;
}
+ rtl_tx_vlan_tag(tx_desc, skb);
+
tx_data += sizeof(*tx_desc);
len = skb->len;
@@ -1691,6 +1714,7 @@ static void rx_bottom(struct r8152 *tp)
memcpy(skb->data, rx_data, pkt_len);
skb_put(skb, pkt_len);
skb->protocol = eth_type_trans(skb, netdev);
+ rtl_rx_vlan_tag(rx_desc, skb);
netif_receive_skb(skb);
stats->rx_packets++;
stats->rx_bytes += pkt_len;
@@ -2082,6 +2106,34 @@ static void r8152_power_cut_en(struct r8152 *tp, bool enable)
ocp_write_word(tp, MCU_TYPE_USB, USB_PM_CTRL_STATUS, ocp_data);
}
+static void rtl_rx_vlan_en(struct r8152 *tp, bool enable)
+{
+ u32 ocp_data;
+
+ ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
+ if (enable)
+ ocp_data |= CPCR_RX_VLAN;
+ else
+ ocp_data &= ~CPCR_RX_VLAN;
+ ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
+}
+
+static int rtl8152_set_features(struct net_device *dev,
+ netdev_features_t features)
+{
+ netdev_features_t changed = features ^ dev->features;
+ struct r8152 *tp = netdev_priv(dev);
+
+ if (changed & NETIF_F_HW_VLAN_CTAG_RX) {
+ if (features & NETIF_F_HW_VLAN_CTAG_RX)
+ rtl_rx_vlan_en(tp, true);
+ else
+ rtl_rx_vlan_en(tp, false);
+ }
+
+ return 0;
+}
+
#define WAKE_ANY (WAKE_PHY | WAKE_MAGIC | WAKE_UCAST | WAKE_BCAST | WAKE_MCAST)
static u32 __rtl_get_wol(struct r8152 *tp)
@@ -2330,9 +2382,7 @@ static void r8152b_exit_oob(struct r8152 *tp)
ocp_write_dword(tp, MCU_TYPE_USB, USB_TX_DMA,
TEST_MODE_DISABLE | TX_SIZE_ADJUST1);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
- ocp_data &= ~CPCR_RX_VLAN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
+ rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8152_RMS);
@@ -2376,9 +2426,7 @@ static void r8152b_enter_oob(struct r8152 *tp)
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8152_RMS);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
- ocp_data |= CPCR_RX_VLAN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
+ rtl_rx_vlan_en(tp, true);
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR);
ocp_data |= ALDPS_PROXY_MODE;
@@ -2532,9 +2580,7 @@ static void r8153_first_init(struct r8152 *tp)
usleep_range(1000, 2000);
}
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
- ocp_data &= ~CPCR_RX_VLAN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
+ rtl_rx_vlan_en(tp, tp->netdev->features & NETIF_F_HW_VLAN_CTAG_RX);
ocp_write_word(tp, MCU_TYPE_PLA, PLA_RMS, RTL8153_RMS);
ocp_write_byte(tp, MCU_TYPE_PLA, PLA_MTPS, MTPS_JUMBO);
@@ -2593,9 +2639,7 @@ static void r8153_enter_oob(struct r8152 *tp)
ocp_data &= ~TEREDO_WAKE_MASK;
ocp_write_word(tp, MCU_TYPE_PLA, PLA_TEREDO_CFG, ocp_data);
- ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_CPCR);
- ocp_data |= CPCR_RX_VLAN;
- ocp_write_word(tp, MCU_TYPE_PLA, PLA_CPCR, ocp_data);
+ rtl_rx_vlan_en(tp, true);
ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PAL_BDC_CR);
ocp_data |= ALDPS_PROXY_MODE;
@@ -3330,6 +3374,7 @@ static const struct net_device_ops rtl8152_netdev_ops = {
.ndo_do_ioctl = rtl8152_ioctl,
.ndo_start_xmit = rtl8152_start_xmit,
.ndo_tx_timeout = rtl8152_tx_timeout,
+ .ndo_set_features = rtl8152_set_features,
.ndo_set_rx_mode = rtl8152_set_rx_mode,
.ndo_set_mac_address = rtl8152_set_mac_address,
.ndo_change_mtu = rtl8152_change_mtu,
@@ -3484,10 +3529,16 @@ static int rtl8152_probe(struct usb_interface *intf,
netdev->features |= NETIF_F_RXCSUM | NETIF_F_IP_CSUM | NETIF_F_SG |
NETIF_F_TSO | NETIF_F_FRAGLIST | NETIF_F_IPV6_CSUM |
- NETIF_F_TSO6;
+ NETIF_F_TSO6 | NETIF_F_HW_VLAN_CTAG_RX |
+ NETIF_F_HW_VLAN_CTAG_TX;
netdev->hw_features = NETIF_F_RXCSUM | NETIF_F_IP_CSUM | NETIF_F_SG |
NETIF_F_TSO | NETIF_F_FRAGLIST |
- NETIF_F_IPV6_CSUM | NETIF_F_TSO6;
+ NETIF_F_IPV6_CSUM | NETIF_F_TSO6 |
+ NETIF_F_HW_VLAN_CTAG_RX |
+ NETIF_F_HW_VLAN_CTAG_TX;
+ netdev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_TSO |
+ NETIF_F_HIGHDMA | NETIF_F_FRAGLIST |
+ NETIF_F_IPV6_CSUM | NETIF_F_TSO6;
netdev->ethtool_ops = &ops;
netif_set_gso_max_size(netdev, RTL_LIMITED_TSO_SIZE);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v11 net-next 00/12] eBPF syscall, verifier, testsuite
From: Alexei Starovoitov @ 2014-09-12 1:29 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Borkmann, David S. Miller, Ingo Molnar, Linus Torvalds,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CALCETrXqwd=dp31fckMPruQMwVw+UAjaf=SSWp8wr_Cdz_tQdw@mail.gmail.com>
On Thu, Sep 11, 2014 at 6:17 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Thu, Sep 11, 2014 at 3:29 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On Thu, Sep 11, 2014 at 2:54 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>
>>>> the verifier log contains full trace. Last unsafe instruction + error
>>>> in many cases is useless. What we found empirically from using
>>>> it over last 2 years is that developers have different learning curve
>>>> to adjust to 'safe' style of C. Pretty much everyone couldn't
>>>> figure out why program is rejected based on last error. Therefore
>>>> verifier emits full log. From the 1st insn all the way till the last
>>>> 'unsafe' instruction. So the log is multiline output.
>>>> 'Understanding eBPF verifier messages' section of
>>>> Documentation/networking/filter.txt provides few trivial
>>>> examples of these multiline messages.
>>>> Like for the program:
>>>> BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>>>> BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>>>> BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>>>> BPF_LD_MAP_FD(BPF_REG_1, 0),
>>>> BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem),
>>>> BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
>>>> BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
>>>> BPF_EXIT_INSN(),
>>>> the verifier log_buf is:
>>>> 0: (7a) *(u64 *)(r10 -8) = 0
>>>> 1: (bf) r2 = r10
>>>> 2: (07) r2 += -8
>>>> 3: (b7) r1 = 0
>>>> 4: (85) call 1
>>>> 5: (15) if r0 == 0x0 goto pc+1
>>>> R0=map_ptr R10=fp
>>>> 6: (7a) *(u64 *)(r0 +4) = 0
>>>> misaligned access off 4 size 8
>>>>
>>>> It will surely change over time as verifier becomes smarter,
>>>> supports new types, optimizations and so on.
>>>> So this log is not an ABI. It's for humans to read.
>>>> The log explains _how_ verifier came to conclusion
>>>> that the program is unsafe.
>>>
>>> Given that you've already arranged (I think) for the verifier to be
>>> compilable in the kernel and in userspace, would it make more sense to
>>> have the kernel version just say yes or no and to make it easy for
>>> user code to retry verification in userspace if they want a full
>>> explanation?
>>
>> Good memory :) Long ago I had a hack where I compiled
>> verifier.o for kernel and linked it with userspace wrappers to
>> have the same verifier for userspace. It was very fragile.
>> and maps were not separate objects and there were no fds.
>> It's not feasible anymore, since different subsystems
>> will configure different bpf_context and helper functions and
>> verifier output is dynamic based on maps that were created.
>> For example, if user's samples/bpf/sock_example.c does
>> bpf_create_map(HASH, sizeof(key) * 2, ...);
>> instead of
>> bpf_create_map(HASH, sizeof(key), ...);
>> the same program will be rejected in first case and will be
>> accepted in the second, because map sizes and ebpf
>> program expectations are mismatching.
>
> Hmm.
>
> This actually furthers my thought that the relocations should be a
> real relocation table. Then you could encode the types of the
> referenced objects in the table, and a program could be verified
> without looking up the fds. The only extra step would be to confirm
> that the actual types referenced match those in the table.
It's not the type is being checked, but one particular map instance
with user specified key/value sizes. type is not helpful. type is not
even used during verification. Only key_size and value_size of
elements are meaningful and they're looked up dynamically by fd.
^ permalink raw reply
* Re: Rusty away 18th September -- 11th October
From: Rusty Russell @ 2014-09-12 1:28 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, Linus Torvalds, LKML, virtualization
In-Reply-To: <20140911114351.GA25736@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Sep 11, 2014 at 10:26:52AM +0930, Rusty Russell wrote:
>> Hi all,
>>
>> Probably won't read mail. Linus, I'll have pull requests early
>> next week; if there's anything needed I'm sure Michael Tsirkin can
>> handle it.
>
> Sure.
> Rusty, there's a small chance virtio 1.0 bits will be ready in time.
> I started working on them based on your virtio-pci-new-layout branch.
>
> If ready before the merge window, are you ok with me merging this
> support (without you having the opportunity to review first)?
Sorry, absolutely not. I *really* want to review this in depth; if we
make a mistake, it's going to hurt us significantly.
And until we have the qemu bits ready, it's really hard to tell if we've
got this right. So I'd rather delay and make sure we're solid.
Thanks,
Rusty.
^ permalink raw reply
* Re: [PATCH v11 net-next 00/12] eBPF syscall, verifier, testsuite
From: Andy Lutomirski @ 2014-09-12 1:17 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Borkmann, David S. Miller, Ingo Molnar, Linus Torvalds,
Steven Rostedt, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuyzOgCZutgsAXs60BO0=0WJGpVAeTYN2hf0Bh1sZ5PMVg@mail.gmail.com>
On Thu, Sep 11, 2014 at 3:29 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On Thu, Sep 11, 2014 at 2:54 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>
>>> the verifier log contains full trace. Last unsafe instruction + error
>>> in many cases is useless. What we found empirically from using
>>> it over last 2 years is that developers have different learning curve
>>> to adjust to 'safe' style of C. Pretty much everyone couldn't
>>> figure out why program is rejected based on last error. Therefore
>>> verifier emits full log. From the 1st insn all the way till the last
>>> 'unsafe' instruction. So the log is multiline output.
>>> 'Understanding eBPF verifier messages' section of
>>> Documentation/networking/filter.txt provides few trivial
>>> examples of these multiline messages.
>>> Like for the program:
>>> BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>>> BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>>> BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>>> BPF_LD_MAP_FD(BPF_REG_1, 0),
>>> BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem),
>>> BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
>>> BPF_ST_MEM(BPF_DW, BPF_REG_0, 4, 0),
>>> BPF_EXIT_INSN(),
>>> the verifier log_buf is:
>>> 0: (7a) *(u64 *)(r10 -8) = 0
>>> 1: (bf) r2 = r10
>>> 2: (07) r2 += -8
>>> 3: (b7) r1 = 0
>>> 4: (85) call 1
>>> 5: (15) if r0 == 0x0 goto pc+1
>>> R0=map_ptr R10=fp
>>> 6: (7a) *(u64 *)(r0 +4) = 0
>>> misaligned access off 4 size 8
>>>
>>> It will surely change over time as verifier becomes smarter,
>>> supports new types, optimizations and so on.
>>> So this log is not an ABI. It's for humans to read.
>>> The log explains _how_ verifier came to conclusion
>>> that the program is unsafe.
>>
>> Given that you've already arranged (I think) for the verifier to be
>> compilable in the kernel and in userspace, would it make more sense to
>> have the kernel version just say yes or no and to make it easy for
>> user code to retry verification in userspace if they want a full
>> explanation?
>
> Good memory :) Long ago I had a hack where I compiled
> verifier.o for kernel and linked it with userspace wrappers to
> have the same verifier for userspace. It was very fragile.
> and maps were not separate objects and there were no fds.
> It's not feasible anymore, since different subsystems
> will configure different bpf_context and helper functions and
> verifier output is dynamic based on maps that were created.
> For example, if user's samples/bpf/sock_example.c does
> bpf_create_map(HASH, sizeof(key) * 2, ...);
> instead of
> bpf_create_map(HASH, sizeof(key), ...);
> the same program will be rejected in first case and will be
> accepted in the second, because map sizes and ebpf
> program expectations are mismatching.
Hmm.
This actually furthers my thought that the relocations should be a
real relocation table. Then you could encode the types of the
referenced objects in the table, and a program could be verified
without looking up the fds. The only extra step would be to confirm
that the actual types referenced match those in the table.
--Andy
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: change tag_protocol to an enum
From: Alexander Duyck @ 2014-09-12 0:46 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: davem, kernel
In-Reply-To: <1410461842-15792-1-git-send-email-f.fainelli@gmail.com>
On 09/11/2014 11:57 AM, Florian Fainelli wrote:
> Now that we introduced an additional multiplexing/demultiplexing layer
> with commit 3e8a72d1dae37 ("net: dsa: reduce number of protocol hooks")
> that lives within the DSA code, we no longer need to have a given switch
> driver tag_protocol be an actual ethertype value, instead, we can
> replace it with an enum: dsa_tag_protocol.
>
> Do this replacement in the drivers, which allows us to get rid of the
> cpu_to_be16()/htons() dance, and remove ETH_P_BRCMTAG since we do not
> need it anymore.
>
> Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/dsa/bcm_sf2.c | 2 +-
> drivers/net/dsa/mv88e6060.c | 2 +-
> drivers/net/dsa/mv88e6123_61_65.c | 4 ++--
> drivers/net/dsa/mv88e6131.c | 2 +-
> include/net/dsa.h | 14 ++++++++++----
> net/dsa/slave.c | 8 ++++----
> net/dsa/tag_brcm.c | 1 -
> 7 files changed, 19 insertions(+), 14 deletions(-)
[...]
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 97712927a9d2..3e88b4ad274f 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -22,7 +22,13 @@
> /* Not an official ethertype value, used only internally for DSA
> * demultiplexing
> */
> -#define ETH_P_BRCMTAG (ETH_P_XDSA + 1)
> +enum dsa_tag_protocol {
> + DSA_TAG_PROTO_NONE = 0,
> + DSA_TAG_PROTO_DSA,
> + DSA_TAG_PROTO_TRAILER,
> + DSA_TAG_PROTO_EDSA,
> + DSA_TAG_PROTO_BRCM,
> +};
>
> #define DSA_MAX_SWITCHES 4
> #define DSA_MAX_PORTS 12
You might want to either remove or update the comment as well. Defining
the enum with a comment about ethertype value might be a bit confusing.
Other than that it looks okay to me.
Thanks,
ALex
^ permalink raw reply
* [PATCH] rtlwifi: btcoexist: avoid format string in printk
From: Larry Finger @ 2014-09-12 0:02 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, Larry Finger, netdev, Kees Cook
Recent changes to this driver inadvertently reverted the change made by Kees
Cook in commit 6437f51ec3.
Reported-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Kees Cook <keescook@chromium.org>
---
drivers/net/wireless/rtlwifi/btcoexist/halbtcoutsrc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/rtlwifi/btcoexist/halbtcoutsrc.h b/drivers/net/wireless/rtlwifi/btcoexist/halbtcoutsrc.h
index 0d185e6..3cd2175 100644
--- a/drivers/net/wireless/rtlwifi/btcoexist/halbtcoutsrc.h
+++ b/drivers/net/wireless/rtlwifi/btcoexist/halbtcoutsrc.h
@@ -117,7 +117,7 @@ extern u32 btc_dbg_type[];
#define WIFI_P2P_GC_CONNECTED BIT4
#define CL_SPRINTF snprintf
-#define CL_PRINTF printk
+#define CL_PRINTF(buf) printk("%s", buf)
#define BTC_PRINT(dbgtype, dbgflag, printstr, ...) \
do { \
--
1.8.4.5
^ permalink raw reply related
* [PATCH net-next v2 4/4] openvswitch: Add recirc and hash action.
From: Pravin B Shelar @ 2014-09-11 23:28 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou, Pravin B Shelar
From: Andy Zhou <azhou@nicira.com>
Recirc action allows a packet to reenter openvswitch processing.
currently openvswitch lookup flow for packet received and execute
set of actions on that packet, with help of recirc action we can
process/modify the packet and recirculate it back in openvswitch
for another pass.
OVS hash action calculates 5-tupple hash and set hash in flow-key
hash. This can be used along with recirculation for distributing
packets among different ports for bond devices.
For example:
OVS bonding can use following actions:
Match on: bond flow; Action: hash, recirc(id)
Match on: recirc-id == id and hash lower bits == a;
Action: output port_bond_a
Signed-off-by: Andy Zhou <azhou@nicira.com>
Acked-by: Jesse Gross <jesse@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
include/uapi/linux/openvswitch.h | 26 +++++
net/openvswitch/actions.c | 207 ++++++++++++++++++++++++++++++++++++++-
net/openvswitch/datapath.c | 11 ++-
net/openvswitch/datapath.h | 6 +-
net/openvswitch/flow.c | 5 +
net/openvswitch/flow.h | 3 +
net/openvswitch/flow_netlink.c | 41 +++++++-
7 files changed, 290 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index a794d1d..f7fc507 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -289,6 +289,9 @@ enum ovs_key_attr {
OVS_KEY_ATTR_TUNNEL, /* Nested set of ovs_tunnel attributes */
OVS_KEY_ATTR_SCTP, /* struct ovs_key_sctp */
OVS_KEY_ATTR_TCP_FLAGS, /* be16 TCP flags. */
+ OVS_KEY_ATTR_DP_HASH, /* u32 hash value. Value 0 indicates the hash
+ is not computed by the datapath. */
+ OVS_KEY_ATTR_RECIRC_ID, /* u32 recirc id */
#ifdef __KERNEL__
OVS_KEY_ATTR_IPV4_TUNNEL, /* struct ovs_key_ipv4_tunnel */
@@ -493,6 +496,27 @@ struct ovs_action_push_vlan {
__be16 vlan_tci; /* 802.1Q TCI (VLAN ID and priority). */
};
+/* Data path hash algorithm for computing Datapath hash.
+ *
+ * The algorithm type only specifies the fields in a flow
+ * will be used as part of the hash. Each datapath is free
+ * to use its own hash algorithm. The hash value will be
+ * opaque to the user space daemon.
+ */
+enum ovs_hash_alg {
+ OVS_HASH_ALG_L4,
+};
+
+/*
+ * struct ovs_action_hash - %OVS_ACTION_ATTR_HASH action argument.
+ * @hash_alg: Algorithm used to compute hash prior to recirculation.
+ * @hash_basis: basis used for computing hash.
+ */
+struct ovs_action_hash {
+ uint32_t hash_alg; /* One of ovs_hash_alg. */
+ uint32_t hash_basis;
+};
+
/**
* enum ovs_action_attr - Action types.
*
@@ -521,6 +545,8 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_PUSH_VLAN, /* struct ovs_action_push_vlan. */
OVS_ACTION_ATTR_POP_VLAN, /* No argument. */
OVS_ACTION_ATTR_SAMPLE, /* Nested OVS_SAMPLE_ATTR_*. */
+ OVS_ACTION_ATTR_RECIRC, /* u32 recirc_id. */
+ OVS_ACTION_ATTR_HASH, /* struct ovs_action_hash. */
__OVS_ACTION_ATTR_MAX
};
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index fda7ef3..52a6a16 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2007-2013 Nicira, Inc.
+ * Copyright (c) 2007-2014 Nicira, Inc.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -35,11 +35,87 @@
#include <net/sctp/checksum.h>
#include "datapath.h"
+#include "flow.h"
#include "vport.h"
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr, int len);
+struct deferred_action {
+ struct sk_buff *skb;
+ const struct nlattr *actions;
+
+ /* Store pkt_key clone when creating deferred action. */
+ struct sw_flow_key pkt_key;
+};
+
+#define DEFERRED_ACTION_FIFO_SIZE 10
+struct action_fifo {
+ int head;
+ int tail;
+ /* Deferred action fifo queue storage. */
+ struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
+};
+
+static struct action_fifo __percpu *action_fifos;
+static DEFINE_PER_CPU(int, exec_actions_level);
+
+static void action_fifo_init(struct action_fifo *fifo)
+{
+ fifo->head = 0;
+ fifo->tail = 0;
+}
+
+static bool action_fifo_is_empty(struct action_fifo *fifo)
+{
+ return (fifo->head == fifo->tail);
+}
+
+static struct deferred_action *action_fifo_get(struct action_fifo *fifo)
+{
+ if (action_fifo_is_empty(fifo))
+ return NULL;
+
+ return &fifo->fifo[fifo->tail++];
+}
+
+static struct deferred_action *action_fifo_put(struct action_fifo *fifo)
+{
+ if (fifo->head >= DEFERRED_ACTION_FIFO_SIZE - 1)
+ return NULL;
+
+ return &fifo->fifo[fifo->head++];
+}
+
+static void flow_key_clone(struct sk_buff *skb, struct sw_flow_key *new_key)
+{
+ *new_key = *OVS_CB(skb)->pkt_key;
+ OVS_CB(skb)->pkt_key = new_key;
+}
+
+/* Return true if fifo is not full */
+static bool add_deferred_actions(struct sk_buff *skb,
+ const struct nlattr *attr)
+{
+ struct action_fifo *fifo;
+ struct deferred_action *da;
+
+ fifo = this_cpu_ptr(action_fifos);
+ da = action_fifo_put(fifo);
+ if (da) {
+ da->skb = skb;
+ da->actions = attr;
+ flow_key_clone(skb, &da->pkt_key);
+ }
+
+ return (da != NULL);
+}
+
+static void flow_key_set_recirc_id(struct sk_buff *skb, u32 recirc_id)
+{
+ OVS_CB(skb)->pkt_key->recirc_id = recirc_id;
+}
+
static int make_writable(struct sk_buff *skb, int write_len)
{
if (!pskb_may_pull(skb, write_len))
@@ -485,8 +561,29 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
/* Skip the sample action when out of memory. */
return 0;
- /* do_execute_actions() will consume the cloned skb. */
- return do_execute_actions(dp, skb, a, rem);
+ if (!add_deferred_actions(skb, a)) {
+ if (net_ratelimit())
+ pr_warn("%s: deferred actions limit reached, dropping sample action\n",
+ ovs_dp_name(dp));
+
+ kfree_skb(skb);
+ }
+ return 0;
+}
+
+static void execute_hash(struct sk_buff *skb, const struct nlattr *attr)
+{
+ struct sw_flow_key *key = OVS_CB(skb)->pkt_key;
+ struct ovs_action_hash *hash_act = nla_data(attr);
+ u32 hash = 0;
+
+ /* OVS_HASH_ALG_L4 is the only possible hash algorithm. */
+ hash = skb_get_hash(skb);
+ hash = jhash_1word(hash, hash_act->hash_basis);
+ if (!hash)
+ hash = 0x1;
+
+ key->ovs_flow_hash = hash;
}
static int execute_set_action(struct sk_buff *skb,
@@ -535,6 +632,41 @@ static int execute_set_action(struct sk_buff *skb,
return err;
}
+static int execute_recirc(struct datapath *dp, struct sk_buff *skb,
+ const struct nlattr *a, int rem)
+{
+ int err;
+
+ err = ovs_flow_key_update(skb, OVS_CB(skb)->pkt_key);
+ if (err)
+ return err;
+
+ if (!last_action(a, rem)) {
+ /* Recirc action is the not the last action
+ * of the action list, need to clone the skb.
+ */
+ skb = skb_clone(skb, GFP_ATOMIC);
+
+ /* Skip the recirc action when out of memory, but
+ * continue on with the rest of the action list.
+ */
+ if (!skb)
+ return 0;
+ }
+
+ if (add_deferred_actions(skb, NULL)) {
+ flow_key_set_recirc_id(skb, nla_get_u32(a));
+ } else {
+ kfree_skb(skb);
+
+ if (net_ratelimit())
+ pr_warn("%s: deferred action limit reached, drop recirc action\n",
+ ovs_dp_name(dp));
+ }
+
+ return 0;
+}
+
/* Execute a list of actions against 'skb'. */
static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
const struct nlattr *attr, int len)
@@ -565,6 +697,10 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
output_userspace(dp, skb, a);
break;
+ case OVS_ACTION_ATTR_HASH:
+ execute_hash(skb, a);
+ break;
+
case OVS_ACTION_ATTR_PUSH_VLAN:
err = push_vlan(skb, nla_data(a));
if (unlikely(err)) /* skb already freed. */
@@ -575,6 +711,17 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
err = pop_vlan(skb);
break;
+ case OVS_ACTION_ATTR_RECIRC:
+ err = execute_recirc(dp, skb, a, rem);
+ if (last_action(a, rem)) {
+ /* If this is the last action, the skb has
+ * been consumed or freed.
+ * Return immediately.
+ */
+ return err;
+ }
+ break;
+
case OVS_ACTION_ATTR_SET:
err = execute_set_action(skb, nla_data(a));
break;
@@ -600,10 +747,60 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
return 0;
}
+static void process_deferred_actions(struct datapath *dp)
+{
+ struct action_fifo *fifo = this_cpu_ptr(action_fifos);
+
+ /* Do not touch the FIFO in case there is no deferred actions. */
+ if (action_fifo_is_empty(fifo))
+ return;
+
+ /* Finishing executing all deferred actions. */
+ do {
+ struct deferred_action *da = action_fifo_get(fifo);
+ struct sk_buff *skb = da->skb;
+ const struct nlattr *actions = da->actions;
+
+ if (actions)
+ do_execute_actions(dp, skb, actions,
+ nla_len(actions));
+ else
+ ovs_dp_process_packet(skb);
+ } while (!action_fifo_is_empty(fifo));
+
+ /* Reset FIFO for the next packet. */
+ action_fifo_init(fifo);
+}
+
/* Execute a list of actions against 'skb'. */
int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb)
{
- struct sw_flow_actions *acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
+ int level = this_cpu_read(exec_actions_level);
+ struct sw_flow_actions *acts;
+ int err;
+
+ acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
+
+ this_cpu_inc(exec_actions_level);
+ err = do_execute_actions(dp, skb, acts->actions, acts->actions_len);
- return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
+ if (!level)
+ process_deferred_actions(dp);
+
+ this_cpu_dec(exec_actions_level);
+ return err;
+}
+
+int action_fifos_init(void)
+{
+ action_fifos = alloc_percpu(struct action_fifo);
+ if (!action_fifos)
+ return -ENOMEM;
+
+ return 0;
+}
+
+void action_fifos_exit(void)
+{
+ free_percpu(action_fifos);
}
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 4046bc2..2409ac0 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -156,7 +156,7 @@ static struct datapath *get_dp(struct net *net, int dp_ifindex)
}
/* Must be called with rcu_read_lock or ovs_mutex. */
-static const char *ovs_dp_name(const struct datapath *dp)
+const char *ovs_dp_name(const struct datapath *dp)
{
struct vport *vport = ovs_vport_ovsl_rcu(dp, OVSP_LOCAL);
return vport->ops->get_name(vport);
@@ -2065,10 +2065,14 @@ static int __init dp_init(void)
pr_info("Open vSwitch switching datapath\n");
- err = ovs_internal_dev_rtnl_link_register();
+ err = action_fifos_init();
if (err)
goto error;
+ err = ovs_internal_dev_rtnl_link_register();
+ if (err)
+ goto error_action_fifos_exit;
+
err = ovs_flow_init();
if (err)
goto error_unreg_rtnl_link;
@@ -2101,6 +2105,8 @@ error_flow_exit:
ovs_flow_exit();
error_unreg_rtnl_link:
ovs_internal_dev_rtnl_link_unregister();
+error_action_fifos_exit:
+ action_fifos_exit();
error:
return err;
}
@@ -2114,6 +2120,7 @@ static void dp_cleanup(void)
ovs_vport_exit();
ovs_flow_exit();
ovs_internal_dev_rtnl_link_unregister();
+ action_fifos_exit();
}
module_init(dp_init);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index fdd96c0..ca162c0 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2007-2012 Nicira, Inc.
+ * Copyright (c) 2007-2014 Nicira, Inc.
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of version 2 of the GNU General Public
@@ -191,12 +191,16 @@ void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
+const char *ovs_dp_name(const struct datapath *dp);
struct sk_buff *ovs_vport_cmd_build_info(struct vport *, u32 pid, u32 seq,
u8 cmd);
int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb);
void ovs_dp_notify_wq(struct work_struct *work);
+int action_fifos_init(void);
+void action_fifos_exit(void);
+
#define OVS_NLERR(fmt, ...) \
do { \
if (net_ratelimit()) \
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 3faffb9..6a600c8 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -608,6 +608,11 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ return key_extract(skb, key);
+}
+
int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
struct sk_buff *skb, struct sw_flow_key *key)
{
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 3869a54..0f5db4e 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -72,6 +72,8 @@ struct sw_flow_key {
u32 skb_mark; /* SKB mark. */
u16 in_port; /* Input switch port (or DP_MAX_PORTS). */
} __packed phy; /* Safe when right after 'tun_key'. */
+ u32 ovs_flow_hash; /* Datapath computed hash value. */
+ u32 recirc_id; /* Recirculation ID. */
struct {
u8 src[ETH_ALEN]; /* Ethernet source address. */
u8 dst[ETH_ALEN]; /* Ethernet destination address. */
@@ -187,6 +189,7 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
+int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key);
int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
struct sk_buff *skb, struct sw_flow_key *key);
/* Extract key from packet coming from userspace. */
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 630b320..61c8add 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -251,6 +251,8 @@ static const int ovs_key_lens[OVS_KEY_ATTR_MAX + 1] = {
[OVS_KEY_ATTR_ICMPV6] = sizeof(struct ovs_key_icmpv6),
[OVS_KEY_ATTR_ARP] = sizeof(struct ovs_key_arp),
[OVS_KEY_ATTR_ND] = sizeof(struct ovs_key_nd),
+ [OVS_KEY_ATTR_RECIRC_ID] = sizeof(u32),
+ [OVS_KEY_ATTR_DP_HASH] = sizeof(u32),
[OVS_KEY_ATTR_TUNNEL] = -1,
};
@@ -454,6 +456,20 @@ static int ipv4_tun_to_nlattr(struct sk_buff *skb,
static int metadata_from_nlattrs(struct sw_flow_match *match, u64 *attrs,
const struct nlattr **a, bool is_mask)
{
+ if (*attrs & (1 << OVS_KEY_ATTR_DP_HASH)) {
+ u32 hash_val = nla_get_u32(a[OVS_KEY_ATTR_DP_HASH]);
+
+ SW_FLOW_KEY_PUT(match, ovs_flow_hash, hash_val, is_mask);
+ *attrs &= ~(1 << OVS_KEY_ATTR_DP_HASH);
+ }
+
+ if (*attrs & (1 << OVS_KEY_ATTR_RECIRC_ID)) {
+ u32 recirc_id = nla_get_u32(a[OVS_KEY_ATTR_RECIRC_ID]);
+
+ SW_FLOW_KEY_PUT(match, recirc_id, recirc_id, is_mask);
+ *attrs &= ~(1 << OVS_KEY_ATTR_RECIRC_ID);
+ }
+
if (*attrs & (1 << OVS_KEY_ATTR_PRIORITY)) {
SW_FLOW_KEY_PUT(match, phy.priority,
nla_get_u32(a[OVS_KEY_ATTR_PRIORITY]), is_mask);
@@ -873,6 +889,12 @@ int ovs_nla_put_flow(const struct sw_flow_key *swkey,
struct nlattr *nla, *encap;
bool is_mask = (swkey != output);
+ if (nla_put_u32(skb, OVS_KEY_ATTR_RECIRC_ID, output->recirc_id))
+ goto nla_put_failure;
+
+ if (nla_put_u32(skb, OVS_KEY_ATTR_DP_HASH, output->ovs_flow_hash))
+ goto nla_put_failure;
+
if (nla_put_u32(skb, OVS_KEY_ATTR_PRIORITY, output->phy.priority))
goto nla_put_failure;
@@ -1401,11 +1423,13 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
/* Expected argument lengths, (u32)-1 for variable length. */
static const u32 action_lens[OVS_ACTION_ATTR_MAX + 1] = {
[OVS_ACTION_ATTR_OUTPUT] = sizeof(u32),
+ [OVS_ACTION_ATTR_RECIRC] = sizeof(u32),
[OVS_ACTION_ATTR_USERSPACE] = (u32)-1,
[OVS_ACTION_ATTR_PUSH_VLAN] = sizeof(struct ovs_action_push_vlan),
[OVS_ACTION_ATTR_POP_VLAN] = 0,
[OVS_ACTION_ATTR_SET] = (u32)-1,
- [OVS_ACTION_ATTR_SAMPLE] = (u32)-1
+ [OVS_ACTION_ATTR_SAMPLE] = (u32)-1,
+ [OVS_ACTION_ATTR_HASH] = sizeof(struct ovs_action_hash)
};
const struct ovs_action_push_vlan *vlan;
int type = nla_type(a);
@@ -1432,6 +1456,18 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
return -EINVAL;
break;
+ case OVS_ACTION_ATTR_HASH: {
+ const struct ovs_action_hash *act_hash = nla_data(a);
+
+ switch (act_hash->hash_alg) {
+ case OVS_HASH_ALG_L4:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ break;
+ }
case OVS_ACTION_ATTR_POP_VLAN:
break;
@@ -1444,6 +1480,9 @@ int ovs_nla_copy_actions(const struct nlattr *attr,
return -EINVAL;
break;
+ case OVS_ACTION_ATTR_RECIRC:
+ break;
+
case OVS_ACTION_ATTR_SET:
err = validate_set(a, key, sfa, &skip_copy);
if (err)
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 3/4] openvswitch: simplify sample action implementation
From: Pravin B Shelar @ 2014-09-11 23:28 UTC (permalink / raw)
To: davem; +Cc: netdev, Andy Zhou, Pravin B Shelar
From: Andy Zhou <azhou@nicira.com>
The current sample() function implementation is more complicated
than necessary in handling single user space action optimization
and skb reference counting. There is no functional changes.
Signed-off-by: Andy Zhou <azhou@nicira.com>
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
---
net/openvswitch/actions.c | 43 ++++++++++++++++++-------------------------
1 file changed, 18 insertions(+), 25 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 39c722f..fda7ef3 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -449,7 +449,6 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
{
const struct nlattr *acts_list = NULL;
const struct nlattr *a;
- struct sk_buff *sample_skb;
int rem;
for (a = nla_data(attr), rem = nla_len(attr); rem > 0;
@@ -467,33 +466,27 @@ static int sample(struct datapath *dp, struct sk_buff *skb,
}
rem = nla_len(acts_list);
- a = nla_data(acts_list);
+ /* Actions list is empty, do nothing */
+ if (unlikely(!rem))
+ return 0;
- /* Actions list is either empty or only contains a single user-space
- * action, the latter being a special case as it is the only known
- * usage of the sample action.
- * In these special cases don't clone the skb as there are no
- * side-effects in the nested actions.
- * Otherwise, clone in case the nested actions have side effects.
+ a = nla_data(acts_list);
+ /* The only known usage of sample action is having a single user-space
+ * action. Treat this usage as a special case.
+ * The output_userspace() should clone the skb to be sent to the
+ * user space. This skb will be consumed by its caller.
*/
- if (likely(rem == 0 || (nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
- last_action(a, rem)))) {
- sample_skb = skb;
- skb_get(skb);
- } else {
- sample_skb = skb_clone(skb, GFP_ATOMIC);
- if (!sample_skb) /* Skip sample action when out of memory. */
- return 0;
- }
+ if (likely(nla_type(a) == OVS_ACTION_ATTR_USERSPACE &&
+ last_action(a, rem)))
+ return output_userspace(dp, skb, a);
- /* Note that do_execute_actions() never consumes skb.
- * In the case where skb has been cloned above it is the clone that
- * is consumed. Otherwise the skb_get(skb) call prevents
- * consumption by do_execute_actions(). Thus, it is safe to simply
- * return the error code and let the caller (also
- * do_execute_actions()) free skb on error.
- */
- return do_execute_actions(dp, sample_skb, a, rem);
+ skb = skb_clone(skb, GFP_ATOMIC);
+ if (!skb)
+ /* Skip the sample action when out of memory. */
+ return 0;
+
+ /* do_execute_actions() will consume the cloned skb. */
+ return do_execute_actions(dp, skb, a, rem);
}
static int execute_set_action(struct sk_buff *skb,
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 2/4] openvswitch: Use tun_key only for egress tunnel path.
From: Pravin B Shelar @ 2014-09-11 23:28 UTC (permalink / raw)
To: davem; +Cc: netdev, Pravin B Shelar
Currently tun_key is used for passing tunnel information
on ingress and egress path, this cause confusion. Following
patch removes its use on ingress path make it egress only parameter.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
---
net/openvswitch/actions.c | 3 +--
net/openvswitch/datapath.c | 19 ++++++-------------
net/openvswitch/datapath.h | 8 ++++----
net/openvswitch/flow.c | 7 ++++---
net/openvswitch/flow.h | 3 ++-
net/openvswitch/vport-gre.c | 22 ++++++++++++----------
net/openvswitch/vport-vxlan.c | 20 +++++++++++---------
net/openvswitch/vport.c | 12 ++++++++++--
8 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 5231652..39c722f 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -511,7 +511,7 @@ static int execute_set_action(struct sk_buff *skb,
break;
case OVS_KEY_ATTR_IPV4_TUNNEL:
- OVS_CB(skb)->tun_key = nla_data(nested_attr);
+ OVS_CB(skb)->egress_tun_key = nla_data(nested_attr);
break;
case OVS_KEY_ATTR_ETHERNET:
@@ -612,6 +612,5 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb)
{
struct sw_flow_actions *acts = rcu_dereference(OVS_CB(skb)->flow->sf_acts);
- OVS_CB(skb)->tun_key = NULL;
return do_execute_actions(dp, skb, acts->actions, acts->actions_len);
}
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 7c3939a..4046bc2 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -237,33 +237,26 @@ void ovs_dp_detach_port(struct vport *p)
}
/* Must be called with rcu_read_lock. */
-void ovs_dp_process_received_packet(struct sk_buff *skb)
+void ovs_dp_process_packet(struct sk_buff *skb)
{
const struct vport *p = OVS_CB(skb)->input_vport;
+ struct sw_flow_key *pkt_key = OVS_CB(skb)->pkt_key;
struct datapath *dp = p->dp;
struct sw_flow *flow;
struct dp_stats_percpu *stats;
- struct sw_flow_key key;
u64 *stats_counter;
u32 n_mask_hit;
- int error;
stats = this_cpu_ptr(dp->stats_percpu);
- /* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_key_extract(skb, &key);
- if (unlikely(error)) {
- kfree_skb(skb);
- return;
- }
-
/* Look up flow. */
- flow = ovs_flow_tbl_lookup_stats(&dp->table, &key, &n_mask_hit);
+ flow = ovs_flow_tbl_lookup_stats(&dp->table, pkt_key, &n_mask_hit);
if (unlikely(!flow)) {
struct dp_upcall_info upcall;
+ int error;
upcall.cmd = OVS_PACKET_CMD_MISS;
- upcall.key = &key;
+ upcall.key = pkt_key;
upcall.userdata = NULL;
upcall.portid = ovs_vport_find_upcall_portid(p, skb);
error = ovs_dp_upcall(dp, skb, &upcall);
@@ -276,7 +269,7 @@ void ovs_dp_process_received_packet(struct sk_buff *skb)
}
OVS_CB(skb)->flow = flow;
- ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
+ ovs_flow_stats_update(OVS_CB(skb)->flow, pkt_key->tp.flags, skb);
ovs_execute_actions(dp, skb);
stats_counter = &stats->n_hit;
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 9d5b7d1..fdd96c0 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -96,15 +96,15 @@ struct datapath {
* struct ovs_skb_cb - OVS data in skb CB
* @flow: The flow associated with this packet. May be %NULL if no flow.
* @pkt_key: The flow information extracted from the packet. Must be nonnull.
- * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
- * packet is not being tunneled.
+ * @egress_tun_key: Tunnel information about this packet on egress path.
+ * NULL if the packet is not being tunneled.
* @input_vport: The original vport packet came in on. This value is cached
* when a packet is received by OVS.
*/
struct ovs_skb_cb {
struct sw_flow *flow;
struct sw_flow_key *pkt_key;
- struct ovs_key_ipv4_tunnel *tun_key;
+ struct ovs_key_ipv4_tunnel *egress_tun_key;
struct vport *input_vport;
};
#define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
@@ -186,7 +186,7 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
-void ovs_dp_process_received_packet(struct sk_buff *);
+void ovs_dp_process_packet(struct sk_buff *);
void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index d264125..3faffb9 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -608,12 +608,13 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
return 0;
}
-int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
+ struct sk_buff *skb, struct sw_flow_key *key)
{
/* Extract metadata from packet. */
memset(key, 0, sizeof(*key));
- if (OVS_CB(skb)->tun_key)
- memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
+ if (tun_key)
+ memcpy(&key->tun_key, tun_key, sizeof(key->tun_key));
key->phy.priority = skb->priority;
key->phy.in_port = OVS_CB(skb)->input_vport->port_no;
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 251789b..3869a54 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -187,7 +187,8 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
-int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key);
+int ovs_flow_key_extract(struct ovs_key_ipv4_tunnel *tun_key,
+ struct sk_buff *skb, struct sw_flow_key *key);
/* Extract key from packet coming from userspace. */
int ovs_flow_key_extract_userspace(const struct nlattr *attr,
struct sk_buff *skb,
diff --git a/net/openvswitch/vport-gre.c b/net/openvswitch/vport-gre.c
index f49148a..e92617b 100644
--- a/net/openvswitch/vport-gre.c
+++ b/net/openvswitch/vport-gre.c
@@ -63,7 +63,7 @@ static __be16 filter_tnl_flags(__be16 flags)
static struct sk_buff *__build_header(struct sk_buff *skb,
int tunnel_hlen)
{
- const struct ovs_key_ipv4_tunnel *tun_key = OVS_CB(skb)->tun_key;
+ const struct ovs_key_ipv4_tunnel *tun_key = OVS_CB(skb)->egress_tun_key;
struct tnl_ptk_info tpi;
skb = gre_handle_offloads(skb, !!(tun_key->tun_flags & TUNNEL_CSUM));
@@ -129,6 +129,7 @@ static int gre_err(struct sk_buff *skb, u32 info,
static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
{
struct net *net = ovs_dp_get_net(vport->dp);
+ struct ovs_key_ipv4_tunnel *tun_key;
struct flowi4 fl;
struct rtable *rt;
int min_headroom;
@@ -136,16 +137,17 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
__be16 df;
int err;
- if (unlikely(!OVS_CB(skb)->tun_key)) {
+ if (unlikely(!OVS_CB(skb)->egress_tun_key)) {
err = -EINVAL;
goto error;
}
+ tun_key = OVS_CB(skb)->egress_tun_key;
/* Route lookup */
memset(&fl, 0, sizeof(fl));
- fl.daddr = OVS_CB(skb)->tun_key->ipv4_dst;
- fl.saddr = OVS_CB(skb)->tun_key->ipv4_src;
- fl.flowi4_tos = RT_TOS(OVS_CB(skb)->tun_key->ipv4_tos);
+ fl.daddr = tun_key->ipv4_dst;
+ fl.saddr = tun_key->ipv4_src;
+ fl.flowi4_tos = RT_TOS(tun_key->ipv4_tos);
fl.flowi4_mark = skb->mark;
fl.flowi4_proto = IPPROTO_GRE;
@@ -153,7 +155,7 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
if (IS_ERR(rt))
return PTR_ERR(rt);
- tunnel_hlen = ip_gre_calc_hlen(OVS_CB(skb)->tun_key->tun_flags);
+ tunnel_hlen = ip_gre_calc_hlen(tun_key->tun_flags);
min_headroom = LL_RESERVED_SPACE(rt->dst.dev) + rt->dst.header_len
+ tunnel_hlen + sizeof(struct iphdr)
@@ -185,15 +187,15 @@ static int gre_tnl_send(struct vport *vport, struct sk_buff *skb)
goto err_free_rt;
}
- df = OVS_CB(skb)->tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
+ df = tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
htons(IP_DF) : 0;
skb->ignore_df = 1;
return iptunnel_xmit(skb->sk, rt, skb, fl.saddr,
- OVS_CB(skb)->tun_key->ipv4_dst, IPPROTO_GRE,
- OVS_CB(skb)->tun_key->ipv4_tos,
- OVS_CB(skb)->tun_key->ipv4_ttl, df, false);
+ tun_key->ipv4_dst, IPPROTO_GRE,
+ tun_key->ipv4_tos,
+ tun_key->ipv4_ttl, df, false);
err_free_rt:
ip_rt_put(rt);
error:
diff --git a/net/openvswitch/vport-vxlan.c b/net/openvswitch/vport-vxlan.c
index d8b7e24..d3f088f 100644
--- a/net/openvswitch/vport-vxlan.c
+++ b/net/openvswitch/vport-vxlan.c
@@ -140,22 +140,24 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
struct net *net = ovs_dp_get_net(vport->dp);
struct vxlan_port *vxlan_port = vxlan_vport(vport);
__be16 dst_port = inet_sk(vxlan_port->vs->sock->sk)->inet_sport;
+ struct ovs_key_ipv4_tunnel *tun_key;
struct rtable *rt;
struct flowi4 fl;
__be16 src_port;
__be16 df;
int err;
- if (unlikely(!OVS_CB(skb)->tun_key)) {
+ if (unlikely(!OVS_CB(skb)->egress_tun_key)) {
err = -EINVAL;
goto error;
}
+ tun_key = OVS_CB(skb)->egress_tun_key;
/* Route lookup */
memset(&fl, 0, sizeof(fl));
- fl.daddr = OVS_CB(skb)->tun_key->ipv4_dst;
- fl.saddr = OVS_CB(skb)->tun_key->ipv4_src;
- fl.flowi4_tos = RT_TOS(OVS_CB(skb)->tun_key->ipv4_tos);
+ fl.daddr = tun_key->ipv4_dst;
+ fl.saddr = tun_key->ipv4_src;
+ fl.flowi4_tos = RT_TOS(tun_key->ipv4_tos);
fl.flowi4_mark = skb->mark;
fl.flowi4_proto = IPPROTO_UDP;
@@ -165,7 +167,7 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
goto error;
}
- df = OVS_CB(skb)->tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
+ df = tun_key->tun_flags & TUNNEL_DONT_FRAGMENT ?
htons(IP_DF) : 0;
skb->ignore_df = 1;
@@ -173,11 +175,11 @@ static int vxlan_tnl_send(struct vport *vport, struct sk_buff *skb)
src_port = udp_flow_src_port(net, skb, 0, 0, true);
err = vxlan_xmit_skb(vxlan_port->vs, rt, skb,
- fl.saddr, OVS_CB(skb)->tun_key->ipv4_dst,
- OVS_CB(skb)->tun_key->ipv4_tos,
- OVS_CB(skb)->tun_key->ipv4_ttl, df,
+ fl.saddr, tun_key->ipv4_dst,
+ tun_key->ipv4_tos,
+ tun_key->ipv4_ttl, df,
src_port, dst_port,
- htonl(be64_to_cpu(OVS_CB(skb)->tun_key->tun_id) << 8),
+ htonl(be64_to_cpu(tun_key->tun_id) << 8),
false);
if (err < 0)
ip_rt_put(rt);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index d21251f..d3f76df 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -435,6 +435,8 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
struct ovs_key_ipv4_tunnel *tun_key)
{
struct pcpu_sw_netstats *stats;
+ struct sw_flow_key key;
+ int error;
stats = this_cpu_ptr(vport->percpu_stats);
u64_stats_update_begin(&stats->syncp);
@@ -442,9 +444,15 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
stats->rx_bytes += skb->len;
u64_stats_update_end(&stats->syncp);
- OVS_CB(skb)->tun_key = tun_key;
OVS_CB(skb)->input_vport = vport;
- ovs_dp_process_received_packet(skb);
+ OVS_CB(skb)->egress_tun_key = NULL;
+ /* Extract flow from 'skb' into 'key'. */
+ error = ovs_flow_key_extract(tun_key, skb, &key);
+ if (unlikely(error)) {
+ kfree_skb(skb);
+ return;
+ }
+ ovs_dp_process_packet(skb);
}
/**
--
1.9.3
^ permalink raw reply related
* [PATCH net-next v2 1/4] openvswitch: refactor ovs flow extract API.
From: Pravin B Shelar @ 2014-09-11 23:28 UTC (permalink / raw)
To: davem; +Cc: netdev, Pravin B Shelar
OVS flow extract is called on packet receive or packet
execute code path. Following patch defines separate API
for extracting flow-key in packet execute code path.
Signed-off-by: Pravin B Shelar <pshelar@nicira.com>
Acked-by: Andy Zhou <azhou@nicira.com>
---
net/openvswitch/datapath.c | 26 +++++++++++++---------
net/openvswitch/datapath.h | 5 ++++-
net/openvswitch/flow.c | 49 +++++++++++++++++++++++++++++++-----------
net/openvswitch/flow.h | 6 +++++-
net/openvswitch/flow_netlink.c | 22 ++++++-------------
net/openvswitch/flow_netlink.h | 4 ++--
net/openvswitch/vport.c | 3 ++-
7 files changed, 72 insertions(+), 43 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index 91d66b7..7c3939a 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -237,8 +237,9 @@ void ovs_dp_detach_port(struct vport *p)
}
/* Must be called with rcu_read_lock. */
-void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
+void ovs_dp_process_received_packet(struct sk_buff *skb)
{
+ const struct vport *p = OVS_CB(skb)->input_vport;
struct datapath *dp = p->dp;
struct sw_flow *flow;
struct dp_stats_percpu *stats;
@@ -250,7 +251,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
stats = this_cpu_ptr(dp->stats_percpu);
/* Extract flow from 'skb' into 'key'. */
- error = ovs_flow_extract(skb, p->port_no, &key);
+ error = ovs_flow_key_extract(skb, &key);
if (unlikely(error)) {
kfree_skb(skb);
return;
@@ -273,9 +274,7 @@ void ovs_dp_process_received_packet(struct vport *p, struct sk_buff *skb)
stats_counter = &stats->n_missed;
goto out;
}
-
OVS_CB(skb)->flow = flow;
- OVS_CB(skb)->pkt_key = &key;
ovs_flow_stats_update(OVS_CB(skb)->flow, key.tp.flags, skb);
ovs_execute_actions(dp, skb);
@@ -340,7 +339,7 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb,
break;
if (skb == segs && gso_type & SKB_GSO_UDP) {
- /* The initial flow key extracted by ovs_flow_extract()
+ /* The initial flow key extracted by ovs-key-extract()
* in this case is for a first fragment, so we need to
* properly mark later fragments.
*/
@@ -515,6 +514,7 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
struct sw_flow *flow;
struct datapath *dp;
struct ethhdr *eth;
+ struct vport *input_vport;
int len;
int err;
@@ -549,13 +549,11 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(flow))
goto err_kfree_skb;
- err = ovs_flow_extract(packet, -1, &flow->key);
+ err = ovs_flow_key_extract_userspace(a[OVS_PACKET_ATTR_KEY], packet,
+ &flow->key);
if (err)
goto err_flow_free;
- err = ovs_nla_get_flow_metadata(flow, a[OVS_PACKET_ATTR_KEY]);
- if (err)
- goto err_flow_free;
acts = ovs_nla_alloc_flow_actions(nla_len(a[OVS_PACKET_ATTR_ACTIONS]));
err = PTR_ERR(acts);
if (IS_ERR(acts))
@@ -568,7 +566,6 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
goto err_flow_free;
OVS_CB(packet)->flow = flow;
- OVS_CB(packet)->pkt_key = &flow->key;
packet->priority = flow->key.phy.priority;
packet->mark = flow->key.phy.skb_mark;
@@ -578,6 +575,15 @@ static int ovs_packet_cmd_execute(struct sk_buff *skb, struct genl_info *info)
if (!dp)
goto err_unlock;
+ input_vport = ovs_vport_rcu(dp, flow->key.phy.in_port);
+ if (!input_vport)
+ input_vport = ovs_vport_rcu(dp, OVSP_LOCAL);
+
+ if (!input_vport)
+ goto err_unlock;
+
+ OVS_CB(packet)->input_vport = input_vport;
+
local_bh_disable();
err = ovs_execute_actions(dp, packet);
local_bh_enable();
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 701b573..9d5b7d1 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -98,11 +98,14 @@ struct datapath {
* @pkt_key: The flow information extracted from the packet. Must be nonnull.
* @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
* packet is not being tunneled.
+ * @input_vport: The original vport packet came in on. This value is cached
+ * when a packet is received by OVS.
*/
struct ovs_skb_cb {
struct sw_flow *flow;
struct sw_flow_key *pkt_key;
struct ovs_key_ipv4_tunnel *tun_key;
+ struct vport *input_vport;
};
#define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
@@ -183,7 +186,7 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
-void ovs_dp_process_received_packet(struct vport *, struct sk_buff *);
+void ovs_dp_process_received_packet(struct sk_buff *);
void ovs_dp_detach_port(struct vport *);
int ovs_dp_upcall(struct datapath *, struct sk_buff *,
const struct dp_upcall_info *);
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 7064da9..d264125 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -16,8 +16,6 @@
* 02110-1301, USA
*/
-#include "flow.h"
-#include "datapath.h"
#include <linux/uaccess.h>
#include <linux/netdevice.h>
#include <linux/etherdevice.h>
@@ -46,6 +44,10 @@
#include <net/ipv6.h>
#include <net/ndisc.h>
+#include "datapath.h"
+#include "flow.h"
+#include "flow_netlink.h"
+
u64 ovs_flow_used_time(unsigned long flow_jiffies)
{
struct timespec cur_ts;
@@ -420,10 +422,9 @@ invalid:
}
/**
- * ovs_flow_extract - extracts a flow key from an Ethernet frame.
+ * key_extract - extracts a flow key from an Ethernet frame.
* @skb: sk_buff that contains the frame, with skb->data pointing to the
* Ethernet header
- * @in_port: port number on which @skb was received.
* @key: output flow key
*
* The caller must ensure that skb->len >= ETH_HLEN.
@@ -442,19 +443,11 @@ invalid:
* of a correct length, otherwise the same as skb->network_header.
* For other key->eth.type values it is left untouched.
*/
-int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
+static int key_extract(struct sk_buff *skb, struct sw_flow_key *key)
{
int error;
struct ethhdr *eth;
- memset(key, 0, sizeof(*key));
-
- key->phy.priority = skb->priority;
- if (OVS_CB(skb)->tun_key)
- memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
- key->phy.in_port = in_port;
- key->phy.skb_mark = skb->mark;
-
skb_reset_mac_header(skb);
/* Link layer. We are guaranteed to have at least the 14 byte Ethernet
@@ -611,5 +604,35 @@ int ovs_flow_extract(struct sk_buff *skb, u16 in_port, struct sw_flow_key *key)
}
}
+ OVS_CB(skb)->pkt_key = key;
return 0;
}
+
+int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key)
+{
+ /* Extract metadata from packet. */
+ memset(key, 0, sizeof(*key));
+ if (OVS_CB(skb)->tun_key)
+ memcpy(&key->tun_key, OVS_CB(skb)->tun_key, sizeof(key->tun_key));
+
+ key->phy.priority = skb->priority;
+ key->phy.in_port = OVS_CB(skb)->input_vport->port_no;
+ key->phy.skb_mark = skb->mark;
+
+ return key_extract(skb, key);
+}
+
+int ovs_flow_key_extract_userspace(const struct nlattr *attr,
+ struct sk_buff *skb,
+ struct sw_flow_key *key)
+{
+ int err;
+
+ memset(key, 0, sizeof(*key));
+ /* Extract metadata from netlink attributes. */
+ err = ovs_nla_get_flow_metadata(attr, key);
+ if (err)
+ return err;
+
+ return key_extract(skb, key);
+}
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index 5e5aaed..251789b 100644
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -187,6 +187,10 @@ void ovs_flow_stats_get(const struct sw_flow *, struct ovs_flow_stats *,
void ovs_flow_stats_clear(struct sw_flow *);
u64 ovs_flow_used_time(unsigned long flow_jiffies);
-int ovs_flow_extract(struct sk_buff *, u16 in_port, struct sw_flow_key *);
+int ovs_flow_key_extract(struct sk_buff *skb, struct sw_flow_key *key);
+/* Extract key from packet coming from userspace. */
+int ovs_flow_key_extract_userspace(const struct nlattr *attr,
+ struct sk_buff *skb,
+ struct sw_flow_key *key);
#endif /* flow.h */
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index d757848..630b320 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -836,7 +836,7 @@ int ovs_nla_get_match(struct sw_flow_match *match,
/**
* ovs_nla_get_flow_metadata - parses Netlink attributes into a flow key.
- * @flow: Receives extracted in_port, priority, tun_key and skb_mark.
+ * @key: Receives extracted in_port, priority, tun_key and skb_mark.
* @attr: Netlink attribute holding nested %OVS_KEY_ATTR_* Netlink attribute
* sequence.
*
@@ -846,32 +846,24 @@ int ovs_nla_get_match(struct sw_flow_match *match,
* extracted from the packet itself.
*/
-int ovs_nla_get_flow_metadata(struct sw_flow *flow,
- const struct nlattr *attr)
+int ovs_nla_get_flow_metadata(const struct nlattr *attr,
+ struct sw_flow_key *key)
{
- struct ovs_key_ipv4_tunnel *tun_key = &flow->key.tun_key;
const struct nlattr *a[OVS_KEY_ATTR_MAX + 1];
+ struct sw_flow_match match;
u64 attrs = 0;
int err;
- struct sw_flow_match match;
-
- flow->key.phy.in_port = DP_MAX_PORTS;
- flow->key.phy.priority = 0;
- flow->key.phy.skb_mark = 0;
- memset(tun_key, 0, sizeof(flow->key.tun_key));
err = parse_flow_nlattrs(attr, a, &attrs);
if (err)
return -EINVAL;
memset(&match, 0, sizeof(match));
- match.key = &flow->key;
+ match.key = key;
- err = metadata_from_nlattrs(&match, &attrs, a, false);
- if (err)
- return err;
+ key->phy.in_port = DP_MAX_PORTS;
- return 0;
+ return metadata_from_nlattrs(&match, &attrs, a, false);
}
int ovs_nla_put_flow(const struct sw_flow_key *swkey,
diff --git a/net/openvswitch/flow_netlink.h b/net/openvswitch/flow_netlink.h
index 4401510..206e45a 100644
--- a/net/openvswitch/flow_netlink.h
+++ b/net/openvswitch/flow_netlink.h
@@ -42,8 +42,8 @@ void ovs_match_init(struct sw_flow_match *match,
int ovs_nla_put_flow(const struct sw_flow_key *,
const struct sw_flow_key *, struct sk_buff *);
-int ovs_nla_get_flow_metadata(struct sw_flow *flow,
- const struct nlattr *attr);
+int ovs_nla_get_flow_metadata(const struct nlattr *, struct sw_flow_key *);
+
int ovs_nla_get_match(struct sw_flow_match *match,
const struct nlattr *,
const struct nlattr *);
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index f7e63f9..d21251f 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -443,7 +443,8 @@ void ovs_vport_receive(struct vport *vport, struct sk_buff *skb,
u64_stats_update_end(&stats->syncp);
OVS_CB(skb)->tun_key = tun_key;
- ovs_dp_process_received_packet(vport, skb);
+ OVS_CB(skb)->input_vport = vport;
+ ovs_dp_process_received_packet(skb);
}
/**
--
1.9.3
^ permalink raw reply related
* [GIT net-next v2] Open vSwitch
From: Pravin B Shelar @ 2014-09-11 23:27 UTC (permalink / raw)
To: davem; +Cc: netdev
Following patches adds recirculation and hash action to OVS.
First three patches does code restructuring which is required
for last patch.
Recirculation implementation is changed, according to comments from
David Miller, to avoid using recursive calls in OVS. It is using
queue to record recirc action and deferred recirc is executed at
the end of current actions execution.
v1-v2:
Changed subsystem name in subject to openvswitch
----------------------------------------------------------------
The following changes since commit b954d83421d51d822c42e5ab7b65069b25ad3005:
net: bpf: only build bpf_jit_binary_{alloc, free}() when jit selected (2014-09-10 14:05:07 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/pshelar/openvswitch.git net_next_ovs
for you to fetch changes up to ee468f5c08e07229ac850e63d74236c3b85f28d5:
openvswitch: Add recirc and hash action. (2014-09-11 16:15:36 -0700)
----------------------------------------------------------------
Andy Zhou (2):
openvswitch: simplify sample action implementation
openvswitch: Add recirc and hash action.
Pravin B Shelar (2):
openvswitch: refactor ovs flow extract API.
openvswitch: Use tun_key only for egress tunnel path.
include/uapi/linux/openvswitch.h | 26 +++++
net/openvswitch/actions.c | 247 ++++++++++++++++++++++++++++++++++-----
net/openvswitch/datapath.c | 52 +++++----
net/openvswitch/datapath.h | 17 ++-
net/openvswitch/flow.c | 55 ++++++---
net/openvswitch/flow.h | 10 +-
net/openvswitch/flow_netlink.c | 63 +++++++---
net/openvswitch/flow_netlink.h | 4 +-
net/openvswitch/vport-gre.c | 22 ++--
net/openvswitch/vport-vxlan.c | 20 ++--
net/openvswitch/vport.c | 13 ++-
11 files changed, 419 insertions(+), 110 deletions(-)
^ permalink raw reply
* [PATCH RFC 6/6] ipv6: switch rt_sernum to atomic_t and clean up types
From: Hannes Frederic Sowa @ 2014-09-11 23:21 UTC (permalink / raw)
To: netdev; +Cc: Eric Dumazet, Vlad Yasevich, Nicolas Dichtel
In-Reply-To: <cover.1410477596.git.hannes@stressinduktion.org>
Switch rt_sernum to atomic_t, make it concurrency safe (the old scheme
looked broken to me) and switch from u32 to int types for the fn_sernum.
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Vlad Yasevich <vyasevich@gmail.com>
Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
---
include/net/ip6_fib.h | 2 +-
include/net/netns/ipv6.h | 2 +-
net/ipv6/af_inet6.c | 2 +-
net/ipv6/ip6_fib.c | 40 ++++++++++++++++++++--------------------
4 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 9bcb220..59c391f 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -64,7 +64,7 @@ struct fib6_node {
__u16 fn_bit; /* bit key */
__u16 fn_flags;
- __u32 fn_sernum;
+ int fn_sernum;
struct rt6_info *rr_ptr;
};
diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h
index 2319949..7dee21b 100644
--- a/include/net/netns/ipv6.h
+++ b/include/net/netns/ipv6.h
@@ -76,7 +76,7 @@ struct netns_ipv6 {
#endif
#endif
atomic_t dev_addr_genid;
- u32 rt_sernum;
+ atomic_t rt_sernum;
};
#if IS_ENABLED(CONFIG_NF_DEFRAG_IPV6)
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 7ff8996..6cde9b4 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -766,7 +766,7 @@ static int __net_init inet6_net_init(struct net *net)
net->ipv6.sysctl.icmpv6_time = 1*HZ;
net->ipv6.sysctl.flowlabel_consistency = 1;
net->ipv6.sysctl.auto_flowlabels = 0;
- net->ipv6.rt_sernum = 1;
+ atomic_set(&net->ipv6.rt_sernum, 1);
err = ipv6_init_mibs(net);
if (err)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 0ffabe2..03234bb 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -60,7 +60,7 @@ struct fib6_cleaner_t {
struct fib6_walker_t w;
struct net *net;
int (*func)(struct rt6_info *, void *arg);
- u32 sernum;
+ int sernum;
void *arg;
};
@@ -73,7 +73,7 @@ static DEFINE_RWLOCK(fib6_walker_lock);
#endif
static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
- u32 sernum);
+ int sernum);
static struct rt6_info *fib6_find_prefix(struct net *net, struct fib6_node *fn);
static struct fib6_node *fib6_repair_tree(struct net *net, struct fib6_node *fn);
static int fib6_walk(struct fib6_walker_t *w);
@@ -105,14 +105,17 @@ static inline void fib6_walker_unlink(struct fib6_walker_t *w)
write_unlock_bh(&fib6_walker_lock);
}
-static u32 fib6_new_sernum(struct net *net)
+static int fib6_new_sernum(struct net *net)
{
- int *n = &net->ipv6.rt_sernum;
+ int old, new;
- (*n)++;
- if ((s32)*n <= 0)
- *n = 1;
- return *n;
+ do {
+ old = atomic_read(&net->ipv6.rt_sernum);
+ new = old + 1;
+ if (new <= 0)
+ new = 1;
+ } while (atomic_cmpxchg(&net->ipv6.rt_sernum, old, new) != old);
+ return new;
}
/*
@@ -427,7 +430,7 @@ static struct fib6_node *fib6_add_1(struct fib6_node *root,
struct in6_addr *addr, int plen,
int offset, int allow_create,
int replace_required,
- u32 sernum)
+ int sernum)
{
struct fib6_node *fn, *in, *ln;
struct fib6_node *pn = NULL;
@@ -851,7 +854,7 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info,
int err = -ENOMEM;
int allow_create = 1;
int replace_required = 0;
- u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
+ int sernum = fib6_new_sernum(dev_net(rt->dst.dev));
if (info->nlh) {
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
@@ -1356,7 +1359,7 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
struct net *net = info->nl_net;
struct fib6_node *fn = rt->rt6i_node;
struct rt6_info **rtp;
- u32 sernum = fib6_new_sernum(dev_net(rt->dst.dev));
+ int sernum = fib6_new_sernum(dev_net(rt->dst.dev));
#if RT6_DEBUG >= 2
if (rt->dst.obsolete > 0) {
@@ -1570,7 +1573,7 @@ static int fib6_clean_node(struct fib6_walker_t *w)
static void fib6_clean_tree(struct net *net, struct fib6_node *root,
int (*func)(struct rt6_info *, void *arg),
- int prune, u32 sernum, void *arg)
+ int prune, int sernum, void *arg)
{
struct fib6_cleaner_t c;
@@ -1589,20 +1592,17 @@ static void fib6_clean_tree(struct net *net, struct fib6_node *root,
static void __fib6_clean_all(struct net *net,
int (*func)(struct rt6_info *, void *arg),
- bool update_sernum, void *arg)
+ int sernum, void *arg)
{
struct fib6_table *table;
struct hlist_head *head;
unsigned int h;
- u32 sernum = 0;
rcu_read_lock();
for (h = 0; h < FIB6_TABLE_HASHSZ; h++) {
head = &net->ipv6.fib_table_hash[h];
hlist_for_each_entry_rcu(table, head, tb6_hlist) {
write_lock_bh(&table->tb6_lock);
- sernum = update_sernum && !sernum ?
- fib6_new_sernum(net) : 0;
fib6_clean_tree(net, &table->tb6_root,
func, 0, sernum, arg);
write_unlock_bh(&table->tb6_lock);
@@ -1614,7 +1614,7 @@ static void __fib6_clean_all(struct net *net,
void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
void *arg)
{
- __fib6_clean_all(net, func, false, arg);
+ __fib6_clean_all(net, func, fib6_new_sernum(net), arg);
}
static int fib6_prune_clone(struct rt6_info *rt, void *arg)
@@ -1628,14 +1628,14 @@ static int fib6_prune_clone(struct rt6_info *rt, void *arg)
}
static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
- u32 sernum)
+ int sernum)
{
fib6_clean_tree(net, fn, fib6_prune_clone, 1, sernum, NULL);
}
static void fib6_flush_trees(struct net *net)
{
- __fib6_clean_all(net, NULL, true, NULL);
+ __fib6_clean_all(net, NULL, fib6_new_sernum(net), NULL);
}
/*
@@ -1846,7 +1846,7 @@ struct ipv6_route_iter {
struct fib6_walker_t w;
loff_t skip;
struct fib6_table *tbl;
- u32 sernum;
+ int sernum;
};
static int ipv6_route_seq_show(struct seq_file *seq, void *v)
--
1.9.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