* [PATCH net 3/3] bonding: remove rtmsg_ifinfo called after bond_lower_state_changed
From: Xin Long @ 2017-10-17 9:39 UTC (permalink / raw)
To: network dev; +Cc: davem, Jiri Pirko
In-Reply-To: <cover.1508233044.git.lucien.xin@gmail.com>
After the patch 'rtnetlink: bring NETDEV_CHANGELOWERSTATE event
process back to rtnetlink_event', bond_lower_state_changed would
generate NETDEV_CHANGEUPPER event which would send a notification
to userspace in rtnetlink_event.
There's no need to call rtmsg_ifinfo to send the notification
any more. So this patch is to remove it from these places after
bond_lower_state_changed.
Besides, after this, rtmsg_ifinfo is not needed to be exported.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/bonding.h | 4 ----
net/core/rtnetlink.c | 1 -
2 files changed, 5 deletions(-)
diff --git a/include/net/bonding.h b/include/net/bonding.h
index b2e6865..1b7631c 100644
--- a/include/net/bonding.h
+++ b/include/net/bonding.h
@@ -330,7 +330,6 @@ static inline void bond_set_active_slave(struct slave *slave)
slave->backup = 0;
bond_queue_slave_event(slave);
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
}
}
@@ -340,7 +339,6 @@ static inline void bond_set_backup_slave(struct slave *slave)
slave->backup = 1;
bond_queue_slave_event(slave);
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
}
}
@@ -353,7 +351,6 @@ static inline void bond_set_slave_state(struct slave *slave,
slave->backup = slave_state;
if (notify) {
bond_lower_state_changed(slave);
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, 0, GFP_ATOMIC);
bond_queue_slave_event(slave);
slave->should_notify = 0;
} else {
@@ -385,7 +382,6 @@ static inline void bond_slave_state_notify(struct bonding *bond)
bond_for_each_slave(bond, tmp, iter) {
if (tmp->should_notify) {
bond_lower_state_changed(tmp);
- rtmsg_ifinfo(RTM_NEWLINK, tmp->dev, 0, GFP_ATOMIC);
tmp->should_notify = 0;
}
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 24cb403..1574ab5 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -2910,7 +2910,6 @@ void rtmsg_ifinfo(int type, struct net_device *dev, unsigned int change,
{
rtmsg_ifinfo_event(type, dev, change, rtnl_get_event(0), flags);
}
-EXPORT_SYMBOL(rtmsg_ifinfo);
static int nlmsg_populate_fdb_fill(struct sk_buff *skb,
struct net_device *dev,
--
2.1.0
^ permalink raw reply related
* [PATCH net 2/3] rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to rtnetlink_event
From: Xin Long @ 2017-10-17 9:39 UTC (permalink / raw)
To: network dev; +Cc: davem, Jiri Pirko
In-Reply-To: <cover.1508233044.git.lucien.xin@gmail.com>
This patch is to bring NETDEV_CHANGELOWERSTATE event process back
to rtnetlink_event so that bonding could use it instead of calling
rtmsg_ifinfo to send a notification to userspace after netdev lower
state is changed in the later patch.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/core/rtnetlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ace489..24cb403 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -4292,6 +4292,7 @@ static int rtnetlink_event(struct notifier_block *this, unsigned long event, voi
case NETDEV_CHANGEUPPER:
case NETDEV_RESEND_IGMP:
case NETDEV_CHANGEINFODATA:
+ case NETDEV_CHANGELOWERSTATE:
case NETDEV_CHANGE_TX_QUEUE_LEN:
rtmsg_ifinfo_event(RTM_NEWLINK, dev, 0, rtnl_get_event(event),
GFP_KERNEL);
--
2.1.0
^ permalink raw reply related
* [PATCH net 1/3] bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link
From: Xin Long @ 2017-10-17 9:39 UTC (permalink / raw)
To: network dev; +Cc: davem, Jiri Pirko
In-Reply-To: <cover.1508233044.git.lucien.xin@gmail.com>
Since commit 42e52bf9e3ae ("net: add netnotifier event for upper device
change"), netdev_master_upper_dev_link has generated NETDEV_CHANGEUPPER
event which would send a notification to userspace in rtnetlink_event.
There's no need to call rtmsg_ifinfo to send the notification any more.
So this patch is to remove it from bond_master_upper_dev_link as well
as bond_upper_dev_unlink to avoid the redundant notifications.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
drivers/net/bonding/bond_main.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index c99dc59..ba75014 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1220,22 +1220,17 @@ static enum netdev_lag_tx_type bond_lag_tx_type(struct bonding *bond)
static int bond_master_upper_dev_link(struct bonding *bond, struct slave *slave)
{
struct netdev_lag_upper_info lag_upper_info;
- int err;
lag_upper_info.tx_type = bond_lag_tx_type(bond);
- err = netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
- &lag_upper_info);
- if (err)
- return err;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
- return 0;
+
+ return netdev_master_upper_dev_link(slave->dev, bond->dev, slave,
+ &lag_upper_info);
}
static void bond_upper_dev_unlink(struct bonding *bond, struct slave *slave)
{
netdev_upper_dev_unlink(slave->dev, bond->dev);
slave->dev->flags &= ~IFF_SLAVE;
- rtmsg_ifinfo(RTM_NEWLINK, slave->dev, IFF_SLAVE, GFP_KERNEL);
}
static struct slave *bond_alloc_slave(struct bonding *bond)
--
2.1.0
^ permalink raw reply related
* [PATCH net 0/3] bonding: void calling rtmsg_ifinfo for netlink notifications
From: Xin Long @ 2017-10-17 9:39 UTC (permalink / raw)
To: network dev; +Cc: davem, Jiri Pirko
It's better to send notifications to userspace by the events
in rtnetlink_event, instead of calling rtmsg_ifinfo directly.
This patcheset is to remove rtmsg_ifinfo called in bonding,
the notifications can be handled by NETDEV_CHANGEUPPER and
NETDEV_CHANGELOWERSTATE events in rtnetlink_event.
It could also fix some redundant notifications from bonding.
Xin Long (3):
bonding: remove rtmsg_ifinfo called in bond_master_upper_dev_link
rtnetlink: bring NETDEV_CHANGELOWERSTATE event process back to
rtnetlink_event
bonding: remove rtmsg_ifinfo called after bond_lower_state_changed
drivers/net/bonding/bond_main.c | 11 +++--------
include/net/bonding.h | 4 ----
net/core/rtnetlink.c | 2 +-
3 files changed, 4 insertions(+), 13 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [PATCH net-next 0/4] bpf: move context info out of the verifier
From: Daniel Borkmann @ 2017-10-17 9:39 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016234056.1964-1-jakub.kicinski@netronome.com>
On 10/17/2017 01:40 AM, Jakub Kicinski wrote:
> Hi!
>
> Daniel pointed out during the review of my previous patchset that
> the knowledge about context doesn't really belong directly in the
> verifier. This patch set takes a bit of a drastic approach to
> move the info out of there. I want to be able to use different
> set of verifier_ops for program analysis. To do that, I have
> to first move the test_run callback to a separate structure. Then
> verifier ops can be declared in the verifier directly and
> different sets can be picked for verification vs analysis.
Looks great, thanks for following up on this!
^ permalink raw reply
* Re: [PATCH net-next 4/4] bpf: allow access to skb->len from offloads
From: Daniel Borkmann @ 2017-10-17 9:38 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016234056.1964-5-jakub.kicinski@netronome.com>
On 10/17/2017 01:40 AM, Jakub Kicinski wrote:
> Since we are now doing strict checking of what offloads
> may access, make sure skb->len is on that list.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next 3/4] bpf: move knowledge about post-translation offsets out of verifier
From: Daniel Borkmann @ 2017-10-17 9:38 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016234056.1964-4-jakub.kicinski@netronome.com>
On 10/17/2017 01:40 AM, Jakub Kicinski wrote:
> Use the fact that verifier ops are now separate from program
> ops to define a separate set of callbacks for verification of
> already translated programs.
>
> Since we expect the analyzer ops to be defined only for
> a small subset of all program types initialize their array
> by hand (don't use linux/bpf_types.h).
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next 2/4] bpf: remove the verifier ops from program structure
From: Daniel Borkmann @ 2017-10-17 9:38 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016234056.1964-3-jakub.kicinski@netronome.com>
On 10/17/2017 01:40 AM, Jakub Kicinski wrote:
> Since the verifier ops don't have to be associated with
> the program for its entire lifetime we can move it to
> verifier's struct bpf_verifier_env.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net-next 1/4] bpf: split verifier and program ops
From: Daniel Borkmann @ 2017-10-17 9:37 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov
In-Reply-To: <20171016234056.1964-2-jakub.kicinski@netronome.com>
On 10/17/2017 01:40 AM, Jakub Kicinski wrote:
> struct bpf_verifier_ops contains both verifier ops and operations
> used later during program's lifetime (test_run). Split the runtime
> ops into a different structure.
>
> BPF_PROG_TYPE() will now append ## _prog_ops or ## _verifier_ops
> to the names.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH] wireless: qtnfmac: Convert timers to use timer_setup()
From: Sergey Matyukevich @ 2017-10-17 8:39 UTC (permalink / raw)
To: Kees Cook
Cc: Kalle Valo, Igor Mitsyanko, Avinash Patil, Sergey Matyukevich,
Kamlesh Rath, linux-wireless, netdev, linux-kernel
In-Reply-To: <20171016233622.GA101550@beast>
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
>
> Cc: Igor Mitsyanko <imitsyanko@quantenna.com>
> Cc: Avinash Patil <avinashp@quantenna.com>
> Cc: Sergey Matyukevich <smatyukevich@quantenna.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: Kamlesh Rath <krath@quantenna.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 7 +++----
> drivers/net/wireless/quantenna/qtnfmac/core.c | 2 +-
> 2 files changed, 4 insertions(+), 5 deletions(-)
Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Thanks!
Sergey
^ permalink raw reply
* Re: [PATCH v2 07/15] usb: gadget: configfs: make config_item_type const
From: Felipe Balbi @ 2017-10-17 8:31 UTC (permalink / raw)
To: Bhumika Goyal, julia.lawall-L2FTfq7BK8M,
rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A,
alexander.shishkin-VuQAYsv1563Yd54FQh9/CA,
jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w, hch-jcswGhMUV9g,
sagi-NQWnxTmZq1alnMjI0IkVqw, kishon-l0cyMroinI0,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, nab-IzHhD5pYlfBP7FQvKIMDCQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
jlbec-aKy9MeLSZ9dg9hUCZPvPmw, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA, mfasheh-rOS7oXVqrJRBDgjK7y7TUQ,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
target-devel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
Cc: Bhumika Goyal
In-Reply-To: <1508167134-6243-8-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 470 bytes --]
Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Make config_item_type structures const as they are either passed to a
> function having the argument as const or stored in the const "ci_type"
> field of a config_item structure.
>
> Done using Coccinelle.
>
> Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Acked-by: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH v2 02/15] usb: gadget: make config_item_type structures const
From: Felipe Balbi @ 2017-10-17 8:30 UTC (permalink / raw)
To: Bhumika Goyal, julia.lawall-L2FTfq7BK8M,
rjw-LthD3rsA81gm4RdzfppkhA, lenb-DgEjT+Ai2ygdnm+yROfE0A,
alexander.shishkin-VuQAYsv1563Yd54FQh9/CA,
jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY,
lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w, hch-jcswGhMUV9g,
sagi-NQWnxTmZq1alnMjI0IkVqw, kishon-l0cyMroinI0,
bhelgaas-hpIqsD4AKlfQT0dZR+AlfA, nab-IzHhD5pYlfBP7FQvKIMDCQ,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw,
jlbec-aKy9MeLSZ9dg9hUCZPvPmw, ccaulfie-H+wXaHxf7aLQT0dZR+AlfA,
teigland-H+wXaHxf7aLQT0dZR+AlfA, mfasheh-rOS7oXVqrJRBDgjK7y7TUQ,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-pci-u79uwXL29TY76Z2rM5mHXA,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
target-devel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
Cc: Bhumika Goyal
In-Reply-To: <1508167134-6243-3-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 575 bytes --]
Hi,
Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> Make these structures const as they are only passed to the const
> argument of the functions config_{group/item}_init_type_name.
>
> Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> * Changes in v2- Combine all the followup patches and the constification
> patches into a series.
I'm assuming this depends on patch 1 of the series. In that case:
Acked-by: Felipe Balbi <felipe.balbi-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/3] dt-bindings: net: sh_eth: add R-Car Gen[12] fallback compatibility strings
From: Geert Uytterhoeven @ 2017-10-17 8:22 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Sergei Shtylyov, Magnus Damm,
netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <20171017074747.24159-2-horms+renesas@verge.net.au>
On Tue, Oct 17, 2017 at 9:47 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Add fallback compatibility strings for R-Car Gen 1 and 2.
>
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
it's
> between IP blocks might be. For example, I believe that r8a7790 is older
> than r8a7791 but that doesn't imply that the latter is a descendant of the
> former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme being adopted for
is being adopted?
> drivers for Renesas SoCs.
>
> Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
> approach that has been consistently taken for other IP blocks is to name
> common code, compatibility strings and so on after Rcar Gen2.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: sh_eth: implement R-Car Gen[12] fallback compatibility strings
From: Geert Uytterhoeven @ 2017-10-17 8:21 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Sergei Shtylyov, Magnus Damm,
netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <20171017074747.24159-4-horms+renesas@verge.net.au>
On Tue, Oct 17, 2017 at 9:47 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Implement fallback compatibility strings for R-Car Gen 1 and 2.
>
> In the case of Renesas R-Car hardware we know that there are generations of
> SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
> between IP blocks might be. For example, I believe that r8a7790 is older
> than r8a7791 but that doesn't imply that the latter is a descendant of the
> former or vice versa.
>
> We can, however, by examining the documentation and behaviour of the
> hardware at run-time observe that the current driver implementation appears
> to be compatible with the IP blocks on SoCs within a given generation.
>
> For the above reasons and convenience when enabling new SoCs a
> per-generation fallback compatibility string scheme being adopted for
is being adopted?
> drivers for Renesas SoCs.
>
> Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
> approach that has been consistently taken for other IP blocks is to name
> common code, compatibility strings and so on after Rcar Gen2.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
^ permalink raw reply
* Re: [PATCH net-next 2/3] net: sh_eth: rename name structures as rcar_gen[12]_*
From: Geert Uytterhoeven @ 2017-10-17 8:20 UTC (permalink / raw)
To: Simon Horman
Cc: David Miller, Sergei Shtylyov, Magnus Damm,
netdev@vger.kernel.org, Linux-Renesas
In-Reply-To: <20171017074747.24159-3-horms+renesas@verge.net.au>
On Tue, Oct 17, 2017 at 9:47 AM, Simon Horman
<horms+renesas@verge.net.au> wrote:
> Rename structures describing R-Car SoCs as rcar_gen[12]_*
> rather than r8a77[79]x_*. This seems a little easier on the
> eyes will make things slightly cleaner in a follow-up
> patch that adds fallback-compatibility strings for these SoCs.
>
> Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
> approach that has been consistently taken for other IP blocks is to name
> common code, compatibility strings and so on after Rcar Gen2.
>
> Also rename sh_eth_set_rate_r8a777x as sh_eth_set_rate_rcar as
> it it is used by the R-Car generations supported by the driver.
>
> This patch should have no run-time effect and
> is compile-tested only.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [net-next RFC 3/4] openvswitch: Add meter infrastructure
From: Andy Zhou @ 2017-10-17 7:40 UTC (permalink / raw)
To: Pravin Shelar; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <CAOrHB_B4JYQ84yMZNHzmbkkgk_R6jukpw=_kQ5S-R8qMq+++Rw@mail.gmail.com>
On Mon, Oct 16, 2017 at 10:49 AM, Pravin Shelar <pshelar@ovn.org> wrote:
> On Mon, Oct 16, 2017 at 12:05 AM, Andy Zhou <azhou@ovn.org> wrote:
>> On Fri, Oct 13, 2017 at 5:12 PM, Pravin Shelar <pshelar@ovn.org> wrote:
>>> On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
>>>> OVS kernel datapath so far does not support Openflow meter action.
>>>> This is the first stab at adding kernel datapath meter support.
>>>> This implementation supports only drop band type.
>>>>
>>>> Signed-off-by: Andy Zhou <azhou@ovn.org>
>>>> ---
>>>> net/openvswitch/Makefile | 1 +
>>>> net/openvswitch/datapath.c | 14 +-
>>>> net/openvswitch/datapath.h | 3 +
>>>> net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++++++++
>>>> net/openvswitch/meter.h | 54 ++++
>>>> 5 files changed, 681 insertions(+), 2 deletions(-)
>>>> create mode 100644 net/openvswitch/meter.c
>>>> create mode 100644 net/openvswitch/meter.h
>>>>
>>> ...
>>>
>>>> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
>>>> new file mode 100644
>>>> index 000000000000..f24ebb5f7af4
>>>> --- /dev/null
>>>> +++ b/net/openvswitch/meter.c
>>>
>>> ....
>>> ....
>>>> +static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
>>>> +{
>>>> + struct datapath *dp;
>>>> + struct ovs_header *ovs_header = info->userhdr;
>>>> + struct sk_buff *reply;
>>>> + struct ovs_header *ovs_reply_header;
>>>> + struct nlattr *nla, *band_nla;
>>>> + int err;
>>>> +
>>>> + /* Check that the datapath exists */
>>>> + ovs_lock();
>>>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>>>> + ovs_unlock();
>>>> + if (!dp)
>>>> + return -ENODEV;
>>>> +
>>> why dp check is required for this API?
>> Is it possible for another core delete the dp, before ovs_lock()
>> returns? Then, in theory, get_dp() can
>> return NULL, no?
>
> I do not see dp used anywhere in function. so the question was why is
> dp lookup done here?
Got it. I misunderstood. You are right, we don't need the dp pointer.
Just posted v2.
>
>>>
>>>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
>>>> + &ovs_reply_header);
>>>> + if (!reply)
>>>> + return PTR_ERR(reply);
>>>> +
>>>> + if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
>>>> + nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
>>>> + goto nla_put_failure;
>>>> +
>>>> + nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
>>>> + if (!nla)
>>>> + goto nla_put_failure;
>>>> +
>>>> + band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
>>>> + if (!band_nla)
>>>> + goto nla_put_failure;
>>>> + /* Currently only DROP band type is supported. */
>>>> + if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
>>>> + goto nla_put_failure;
>>>> + nla_nest_end(reply, band_nla);
>>>> + nla_nest_end(reply, nla);
>>>> +
>>>> + genlmsg_end(reply, ovs_reply_header);
>>>> + return genlmsg_reply(reply, info);
>>>> +
>>>> +nla_put_failure:
>>>> + nlmsg_free(reply);
>>>> + err = -EMSGSIZE;
>>>> + return err;
>>>> +}
>>>> +
>>> ....
>>>
>>>> +static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
>>>> +{
>>>> + struct nlattr **a = info->attrs;
>>>> + struct dp_meter *meter, *old_meter;
>>>> + struct sk_buff *reply;
>>>> + struct ovs_header *ovs_reply_header;
>>>> + struct ovs_header *ovs_header = info->userhdr;
>>>> + struct datapath *dp;
>>>> + int err;
>>>> + u32 meter_id;
>>>> + bool failed;
>>>> +
>>>> + meter = dp_meter_create(a);
>>>> + if (IS_ERR_OR_NULL(meter))
>>>> + return PTR_ERR(meter);
>>>> +
>>>> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
>>>> + &ovs_reply_header);
>>>> + if (IS_ERR(reply)) {
>>>> + err = PTR_ERR(reply);
>>>> + goto exit_free_meter;
>>>> + }
>>>> +
>>>> + ovs_lock();
>>>> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>>>> + if (!dp) {
>>>> + err = -ENODEV;
>>>> + goto exit_unlock;
>>>> + }
>>>> +
>>>> + if (!a[OVS_METER_ATTR_ID]) {
>>>> + err = -ENODEV;
>>>> + goto exit_unlock;
>>>> + }
>>>> +
>>>> + meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>>>> +
>>>> + /* Cannot fail after this. */
>>>> + old_meter = lookup_meter(dp, meter_id);
>>>> + attach_meter(dp, meter);
>>>> + ovs_unlock();
>>>> +
>>> After the unlock, it is not safe to keep the ref to old_meter. better
>>> to release lock at the end. we could optimize it later if required.
>>
>> I see a problem here: the old_meter has not been removed from the list before
>> unlock. The code should have been:
>>
>> old_meter = lookup_meter(dp, meter_id);
>> detch_meter(dp, old_meter);
>> attach_meter(dp, meter);
>> ovs_unlock();
>>
>> Do you still see a problem w.r.t. unlock() here? Once detch_meter() is called,
>> another thread should not have access to 'old_meter' any more. right?
>
> looks good.
^ permalink raw reply
* Re: Ethtool question
From: Gal Pressman @ 2017-10-17 7:49 UTC (permalink / raw)
To: Ben Greear, Roopa Prabhu
Cc: David Miller, John W. Linville, netdev@vger.kernel.org
In-Reply-To: <fec121f7-67bb-486f-ffc0-59f292614043@candelatech.com>
On 16-Oct-17 21:10, Ben Greear wrote:
> On 10/12/2017 03:00 PM, Roopa Prabhu wrote:
>> On Thu, Oct 12, 2017 at 2:45 PM, Ben Greear <greearb@candelatech.com> wrote:
>>> On 10/11/2017 01:49 PM, David Miller wrote:
>>>>
>>>> From: "John W. Linville" <linville@tuxdriver.com>
>>>> Date: Wed, 11 Oct 2017 16:44:07 -0400
>>>>
>>>>> On Wed, Oct 11, 2017 at 09:51:56AM -0700, Ben Greear wrote:
>>>>>>
>>>>>> I noticed today that setting some ethtool settings to the same value
>>>>>> returns an error code. I would think this should silently return
>>>>>> success instead? Makes it easier to call it from scripts this way:
>>>>>>
>>>>>> [root@lf0313-6477 lanforge]# ethtool -L eth3 combined 1
>>>>>> combined unmodified, ignoring
>>>>>> no channel parameters changed, aborting
>>>>>> current values: tx 0 rx 0 other 1 combined 1
>>>>>> [root@lf0313-6477 lanforge]# echo $?
>>>>>> 1
>>>>>
>>>>>
>>>>> I just had this discussion a couple of months ago with someone. My
>>>>> initial feeling was like you, a no-op is not a failure. But someone
>>>>> convinced me otherwise...I will now endeavour to remember who that
>>>>> was and how they convinced me...
>>>>>
>>>>> Anyone else have input here?
>>>>
>>>>
>>>> I guess this usually happens when drivers don't support changing the
>>>> settings at all. So they just make their ethtool operation for the
>>>> 'set' always return an error.
>>>>
>>>> We could have a generic ethtool helper that does "get" and then if the
>>>> "set" request is identical just return zero.
>>>>
>>>> But from another perspective, the error returned from the "set" in this
>>>> situation also indicates to the user that the driver does not support
>>>> the "set" operation which has value and meaning in and of itself. And
>>>> we'd lose that with the given suggestion.
>>>
>>>
>>> In my case, the driver (igb) does support the set, my program just made the
>>> same
>>> ethtool call several times and it fails after the initial change (that
>>> actually
>>> changes something), as best as I can figure.
>>
>>
>> This error is returned by ethtool user-space. It does a get, check and
>> then set if user has requested changes.
>>
>
> So, should we fix ethtool to return 0 in this case instead of an error code?
>
> I think so. If the driver itself returns an error, then probably return the
> error code and/or fix the driver as seems appropriate.
>
> Thanks,
> Ben
>
FWIW, seems like the code isn't consistent in this matter.
Setting pause/channels/coalescing with the same values will return an error code, but setting EEE will return success.
^ permalink raw reply
* [PATCH net-next 2/3] net: sh_eth: rename name structures as rcar_gen[12]_*
From: Simon Horman @ 2017-10-17 7:47 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Simon Horman
In-Reply-To: <20171017074747.24159-1-horms+renesas@verge.net.au>
Rename structures describing R-Car SoCs as rcar_gen[12]_*
rather than r8a77[79]x_*. This seems a little easier on the
eyes will make things slightly cleaner in a follow-up
patch that adds fallback-compatibility strings for these SoCs.
Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
approach that has been consistently taken for other IP blocks is to name
common code, compatibility strings and so on after Rcar Gen2.
Also rename sh_eth_set_rate_r8a777x as sh_eth_set_rate_rcar as
it it is used by the R-Car generations supported by the driver.
This patch should have no run-time effect and
is compile-tested only.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/net/ethernet/renesas/sh_eth.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index d2e88a30f57b..c9f92fc8555e 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -594,7 +594,7 @@ static struct sh_eth_cpu_data r8a7740_data = {
};
/* There is CPU dependent code */
-static void sh_eth_set_rate_r8a777x(struct net_device *ndev)
+static void sh_eth_set_rate_rcar(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
@@ -608,10 +608,10 @@ static void sh_eth_set_rate_r8a777x(struct net_device *ndev)
}
}
-/* R8A7778/9 */
-static struct sh_eth_cpu_data r8a777x_data = {
+/* R-Car Gen1 */
+static struct sh_eth_cpu_data rcar_gen1_data = {
.set_duplex = sh_eth_set_duplex,
- .set_rate = sh_eth_set_rate_r8a777x,
+ .set_rate = sh_eth_set_rate_rcar,
.register_type = SH_ETH_REG_FAST_RCAR,
@@ -635,10 +635,10 @@ static struct sh_eth_cpu_data r8a777x_data = {
.hw_swap = 1,
};
-/* R8A7790/1 */
-static struct sh_eth_cpu_data r8a779x_data = {
+/* R-Car Gen2 and RZ/G1 */
+static struct sh_eth_cpu_data rcar_gen2_data = {
.set_duplex = sh_eth_set_duplex,
- .set_rate = sh_eth_set_rate_r8a777x,
+ .set_rate = sh_eth_set_rate_rcar,
.register_type = SH_ETH_REG_FAST_RCAR,
@@ -3086,14 +3086,14 @@ static struct sh_eth_plat_data *sh_eth_parse_dt(struct device *dev)
static const struct of_device_id sh_eth_match_table[] = {
{ .compatible = "renesas,gether-r8a7740", .data = &r8a7740_data },
- { .compatible = "renesas,ether-r8a7743", .data = &r8a779x_data },
- { .compatible = "renesas,ether-r8a7745", .data = &r8a779x_data },
- { .compatible = "renesas,ether-r8a7778", .data = &r8a777x_data },
- { .compatible = "renesas,ether-r8a7779", .data = &r8a777x_data },
- { .compatible = "renesas,ether-r8a7790", .data = &r8a779x_data },
- { .compatible = "renesas,ether-r8a7791", .data = &r8a779x_data },
- { .compatible = "renesas,ether-r8a7793", .data = &r8a779x_data },
- { .compatible = "renesas,ether-r8a7794", .data = &r8a779x_data },
+ { .compatible = "renesas,ether-r8a7743", .data = &rcar_gen2_data },
+ { .compatible = "renesas,ether-r8a7745", .data = &rcar_gen2_data },
+ { .compatible = "renesas,ether-r8a7778", .data = &rcar_gen1_data },
+ { .compatible = "renesas,ether-r8a7779", .data = &rcar_gen1_data },
+ { .compatible = "renesas,ether-r8a7790", .data = &rcar_gen2_data },
+ { .compatible = "renesas,ether-r8a7791", .data = &rcar_gen2_data },
+ { .compatible = "renesas,ether-r8a7793", .data = &rcar_gen2_data },
+ { .compatible = "renesas,ether-r8a7794", .data = &rcar_gen2_data },
{ .compatible = "renesas,ether-r7s72100", .data = &r7s72100_data },
{ }
};
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 1/3] dt-bindings: net: sh_eth: add R-Car Gen[12] fallback compatibility strings
From: Simon Horman @ 2017-10-17 7:47 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Simon Horman
In-Reply-To: <20171017074747.24159-1-horms+renesas@verge.net.au>
Add fallback compatibility strings for R-Car Gen 1 and 2.
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
between IP blocks might be. For example, I believe that r8a7790 is older
than r8a7791 but that doesn't imply that the latter is a descendant of the
former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme being adopted for
drivers for Renesas SoCs.
Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
approach that has been consistently taken for other IP blocks is to name
common code, compatibility strings and so on after Rcar Gen2.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
Documentation/devicetree/bindings/net/sh_eth.txt | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/sh_eth.txt b/Documentation/devicetree/bindings/net/sh_eth.txt
index 0115c85a2425..48cab94dd056 100644
--- a/Documentation/devicetree/bindings/net/sh_eth.txt
+++ b/Documentation/devicetree/bindings/net/sh_eth.txt
@@ -4,7 +4,8 @@ This file provides information on what the device node for the SH EtherMAC
interface contains.
Required properties:
-- compatible: "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
+- compatible: Must contain one or more of the following:
+ "renesas,gether-r8a7740" if the device is a part of R8A7740 SoC.
"renesas,ether-r8a7743" if the device is a part of R8A7743 SoC.
"renesas,ether-r8a7745" if the device is a part of R8A7745 SoC.
"renesas,ether-r8a7778" if the device is a part of R8A7778 SoC.
@@ -14,6 +15,14 @@ Required properties:
"renesas,ether-r8a7793" if the device is a part of R8A7793 SoC.
"renesas,ether-r8a7794" if the device is a part of R8A7794 SoC.
"renesas,ether-r7s72100" if the device is a part of R7S72100 SoC.
+ "renesas,rcar-gen1-ether" for a generic R-Car Gen1 device.
+ "renesas,rcar-gen2-ether" for a generic R-Car Gen2 or RZ/G1
+ device.
+
+ When compatible with the generic version nodes must list
+ the SoC-specific version corresponding to the platform
+ first followed by the generic version.
+
- reg: offset and length of (1) the E-DMAC/feLic register block (required),
(2) the TSU register block (optional).
- interrupts: interrupt specifier for the sole interrupt.
@@ -36,7 +45,8 @@ Optional properties:
Example (Lager board):
ethernet@ee700000 {
- compatible = "renesas,ether-r8a7790";
+ compatible = "renesas,ether-r8a7790",
+ "renesas,rcar-gen2-ether";
reg = <0 0xee700000 0 0x400>;
interrupt-parent = <&gic>;
interrupts = <0 162 IRQ_TYPE_LEVEL_HIGH>;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/3] net: sh_eth: implement R-Car Gen[12] fallback compatibility strings
From: Simon Horman @ 2017-10-17 7:47 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Simon Horman
In-Reply-To: <20171017074747.24159-1-horms+renesas@verge.net.au>
Implement fallback compatibility strings for R-Car Gen 1 and 2.
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
between IP blocks might be. For example, I believe that r8a7790 is older
than r8a7791 but that doesn't imply that the latter is a descendant of the
former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme being adopted for
drivers for Renesas SoCs.
Note that R-Car Gen2 and RZ/G1 have many compatible IP blocks. The
approach that has been consistently taken for other IP blocks is to name
common code, compatibility strings and so on after Rcar Gen2.
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
drivers/net/ethernet/renesas/sh_eth.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index c9f92fc8555e..7e060aa9fbed 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -3095,6 +3095,8 @@ static const struct of_device_id sh_eth_match_table[] = {
{ .compatible = "renesas,ether-r8a7793", .data = &rcar_gen2_data },
{ .compatible = "renesas,ether-r8a7794", .data = &rcar_gen2_data },
{ .compatible = "renesas,ether-r7s72100", .data = &r7s72100_data },
+ { .compatible = "renesas,rcar-gen1-ether", .data = &rcar_gen1_data },
+ { .compatible = "renesas,rcar-gen2-ether", .data = &rcar_gen2_data },
{ }
};
MODULE_DEVICE_TABLE(of, sh_eth_match_table);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 0/3] net: sh_eth: add R-Car Gen[12] fallback compatibility strings
From: Simon Horman @ 2017-10-17 7:47 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Simon Horman
Add fallback compatibility strings for R-Car Gen 1 and 2.
In the case of Renesas R-Car hardware we know that there are generations of
SoCs, f.e. Gen 1 and 2. But beyond that its not clear what the relationship
between IP blocks might be. For example, I believe that r8a7790 is older
than r8a7791 but that doesn't imply that the latter is a descendant of the
former or vice versa.
We can, however, by examining the documentation and behaviour of the
hardware at run-time observe that the current driver implementation appears
to be compatible with the IP blocks on SoCs within a given generation.
For the above reasons and convenience when enabling new SoCs a
per-generation fallback compatibility string scheme being adopted for
drivers for Renesas SoCs.
Simon Horman (3):
dt-bindings: net: sh_eth: add R-Car Gen[12] fallback compatibility
strings
net: sh_eth: rename name structures as rcar_gen[12]_*
net: sh_eth: implement R-Car Gen[12] fallback compatibility strings
Documentation/devicetree/bindings/net/sh_eth.txt | 14 +++++++++--
drivers/net/ethernet/renesas/sh_eth.c | 32 +++++++++++++-----------
2 files changed, 29 insertions(+), 17 deletions(-)
--
2.11.0
^ permalink raw reply
* Re: [patch net-next 06/34] net: core: use dev->ingress_queue instead of tp->q
From: Jiri Pirko @ 2017-10-17 7:42 UTC (permalink / raw)
To: Daniel Borkmann
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, dsahern,
alexander.h.duyck, john.fastabend, willemb
In-Reply-To: <59E51474.2050708@iogearbox.net>
Mon, Oct 16, 2017 at 10:20:04PM CEST, daniel@iogearbox.net wrote:
>On 10/15/2017 08:45 AM, Jiri Pirko wrote:
>> Sun, Oct 15, 2017 at 01:18:54AM CEST, daniel@iogearbox.net wrote:
>> > On 10/13/2017 08:30 AM, Jiri Pirko wrote:
>> > > Thu, Oct 12, 2017 at 11:45:43PM CEST, daniel@iogearbox.net wrote:
>> > > > On 10/12/2017 07:17 PM, Jiri Pirko wrote:
>> > > > > From: Jiri Pirko <jiri@mellanox.com>
>> > > > >
>> > > > > In sch_handle_egress and sch_handle_ingress, don't use tp->q and use
>> > > > > dev->ingress_queue which stores the same pointer instead.
>> > > > >
>> > > > > Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> > > > > ---
>> > > > > net/core/dev.c | 21 +++++++++++++++------
>> > > > > 1 file changed, 15 insertions(+), 6 deletions(-)
>> > > > >
>> > > > > diff --git a/net/core/dev.c b/net/core/dev.c
>> > > > > index fcddccb..cb9e5e5 100644
>> > > > > --- a/net/core/dev.c
>> > > > > +++ b/net/core/dev.c
>> > > > > @@ -3273,14 +3273,18 @@ EXPORT_SYMBOL(dev_loopback_xmit);
>> > > > > static struct sk_buff *
>> > > > > sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
>> > > > > {
>> > > > > + struct netdev_queue *netdev_queue =
>> > > > > + rcu_dereference_bh(dev->ingress_queue);
>> > > > > struct tcf_proto *cl = rcu_dereference_bh(dev->egress_cl_list);
>> > > > > struct tcf_result cl_res;
>> > > > > + struct Qdisc *q;
>> > > > >
>> > > > > - if (!cl)
>> > > > > + if (!cl || !netdev_queue)
>> > > > > return skb;
>> > > > > + q = netdev_queue->qdisc;
>> > > >
>> > > > NAK, no additional overhead in the software fast-path of
>> > > > sch_handle_{ingress,egress}() like this. There are users out there
>> > > > that use tc in software only, so performance is critical here.
>> > >
>> > > Okay, how else do you suggest I can avoid the need to use tp->q?
>> > > I was thinking about storing q directly to net_device, which would safe
>> > > one dereference, resulting in the same amount as current cl->q.
>> >
>> > Sorry for late reply, mostly off for few days. netdev struct has different
>> > cachelines which are hot on rx and tx (see also the location of the two
>> > lists, ingress_cl_list and egress_cl_list), if you add only one qdisc
>> > pointer there, then you'd at least penalize one of the two w/ potential
>> > cache miss. Can we leave it in cl?
>>
>> Well that is the whole point of this excercise, to remove it from cl.
>> The thing is, for the shared blocks, cls are shared between multiple
>> qdisc instances, so cl->q makes no longer any sense.
>
>I don't really mind if you want to remove cl->q, but lets do it without
>adding any cost to the fast path. The primary motivation for this set
>is hw offload, but it shouldn't be at the expense to make sw fast path
That is not precise. The reason is not only hw offload. User could use
the block sharing for pure sw solution as well and benefit from that.
>slower. cl->q is only used here for updating stats, one possible option
>could be to take them out for the clsact case into dev at the expense
>for two pointers (e.g. getting rid of ingress_queue would reduce it to
>only one pointer), such that you have percpu {ingress,egress}_cl_stats
>that control path can fetch instead when dumping qdiscs. Could be one,
>but there are probably other options as well to explore.
I'm currently looking at that. Thanks.
^ permalink raw reply
* [net-next v2 4/4] openvswitch: Add meter action support
From: Andy Zhou @ 2017-10-17 7:36 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1508225805-11613-1-git-send-email-azhou@ovn.org>
Implements OVS kernel meter action support.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
include/uapi/linux/openvswitch.h | 3 +++
net/openvswitch/actions.c | 6 ++++++
net/openvswitch/datapath.h | 1 +
net/openvswitch/flow_netlink.c | 6 ++++++
4 files changed, 16 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index d413a2398214..de2807a9cc60 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -808,6 +808,8 @@ struct ovs_action_push_eth {
* @OVS_ACTION_ATTR_POP_ETH: Pop the outermost Ethernet header off the
* packet.
* @OVS_ACTION_ATTR_CT_CLEAR: Clear conntrack state from the packet.
+ * @OVS_ACTION_ATTR_METER: Run packet through a meter, which may drop the
+ * packet, or modify the packet (e.g., change the DSCP field).
*
* Only a single header can be set with a single %OVS_ACTION_ATTR_SET. Not all
* fields within a header are modifiable, e.g. the IPv4 protocol and fragment
@@ -838,6 +840,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
OVS_ACTION_ATTR_POP_ETH, /* No argument. */
OVS_ACTION_ATTR_CT_CLEAR, /* No argument. */
+ OVS_ACTION_ATTR_METER, /* u32 meter ID. */
__OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
* from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index a551232daf61..df342fe66492 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1214,6 +1214,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
case OVS_ACTION_ATTR_POP_ETH:
err = pop_eth(skb, key);
break;
+
+ case OVS_ACTION_ATTR_METER:
+ if (ovs_meter_execute(dp, skb, key, nla_get_u32(a))) {
+ consume_skb(skb);
+ return 0;
+ }
}
if (unlikely(err)) {
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index d1ffa1d9fe57..cda40c6af40a 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -30,6 +30,7 @@
#include "conntrack.h"
#include "flow.h"
#include "flow_table.h"
+#include "meter.h"
#include "vport-internal_dev.h"
#define DP_MAX_PORTS USHRT_MAX
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index dc0d79092e74..6789bb1ddf0e 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -87,6 +87,7 @@ static bool actions_may_change_flow(const struct nlattr *actions)
case OVS_ACTION_ATTR_SAMPLE:
case OVS_ACTION_ATTR_SET:
case OVS_ACTION_ATTR_SET_MASKED:
+ case OVS_ACTION_ATTR_METER:
default:
return true;
}
@@ -2533,6 +2534,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
[OVS_ACTION_ATTR_TRUNC] = sizeof(struct ovs_action_trunc),
[OVS_ACTION_ATTR_PUSH_ETH] = sizeof(struct ovs_action_push_eth),
[OVS_ACTION_ATTR_POP_ETH] = 0,
+ [OVS_ACTION_ATTR_METER] = sizeof(u32),
};
const struct ovs_action_push_vlan *vlan;
int type = nla_type(a);
@@ -2690,6 +2692,10 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
mac_proto = MAC_PROTO_ETHERNET;
break;
+ case OVS_ACTION_ATTR_METER:
+ /* Non-existent meters are simply ignored. */
+ break;
+
default:
OVS_NLERR(log, "Unknown Action type %d", type);
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related
* [net-next v2 3/4] openvswitch: Add meter infrastructure
From: Andy Zhou @ 2017-10-17 7:36 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1508225805-11613-1-git-send-email-azhou@ovn.org>
OVS kernel datapath so far does not support Openflow meter action.
This is the first stab at adding kernel datapath meter support.
This implementation supports only drop band type.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
net/openvswitch/Makefile | 1 +
net/openvswitch/datapath.c | 14 +-
net/openvswitch/datapath.h | 3 +
net/openvswitch/meter.c | 604 +++++++++++++++++++++++++++++++++++++++++++++
net/openvswitch/meter.h | 54 ++++
5 files changed, 674 insertions(+), 2 deletions(-)
create mode 100644 net/openvswitch/meter.c
create mode 100644 net/openvswitch/meter.h
diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
index 60f809085b92..658383fbdf53 100644
--- a/net/openvswitch/Makefile
+++ b/net/openvswitch/Makefile
@@ -11,6 +11,7 @@ openvswitch-y := \
flow.o \
flow_netlink.o \
flow_table.o \
+ meter.o \
vport.o \
vport-internal_dev.o \
vport-netdev.o
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ac7154018676..eef8d3ea3aae 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -55,6 +55,7 @@
#include "flow.h"
#include "flow_table.h"
#include "flow_netlink.h"
+#include "meter.h"
#include "vport-internal_dev.h"
#include "vport-netdev.h"
@@ -174,6 +175,7 @@ static void destroy_dp_rcu(struct rcu_head *rcu)
ovs_flow_tbl_destroy(&dp->table);
free_percpu(dp->stats_percpu);
kfree(dp->ports);
+ ovs_meters_exit(dp);
kfree(dp);
}
@@ -1572,6 +1574,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
INIT_HLIST_HEAD(&dp->ports[i]);
+ err = ovs_meters_init(dp);
+ if (err)
+ goto err_destroy_ports_array;
+
/* Set up our datapath device. */
parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
parms.type = OVS_VPORT_TYPE_INTERNAL;
@@ -1600,7 +1606,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
- goto err_destroy_ports_array;
+ goto err_destroy_meters;
}
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
@@ -1615,8 +1621,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
ovs_notify(&dp_datapath_genl_family, reply, info);
return 0;
-err_destroy_ports_array:
+err_destroy_meters:
ovs_unlock();
+ ovs_meters_exit(dp);
+err_destroy_ports_array:
kfree(dp->ports);
err_destroy_percpu:
free_percpu(dp->stats_percpu);
@@ -2244,6 +2252,7 @@ struct genl_family dp_vport_genl_family __ro_after_init = {
&dp_vport_genl_family,
&dp_flow_genl_family,
&dp_packet_genl_family,
+ &dp_meter_genl_family,
};
static void dp_unregister_genl(int n_families)
@@ -2424,3 +2433,4 @@ static void dp_cleanup(void)
MODULE_ALIAS_GENL_FAMILY(OVS_VPORT_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_FLOW_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_PACKET_FAMILY);
+MODULE_ALIAS_GENL_FAMILY(OVS_METER_FAMILY);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index ad14b571219d..d1ffa1d9fe57 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -92,6 +92,9 @@ struct datapath {
u32 user_features;
u32 max_headroom;
+
+ /* Switch meters. */
+ struct hlist_head *meters;
};
/**
diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
new file mode 100644
index 000000000000..ff57c1cb5f6a
--- /dev/null
+++ b/net/openvswitch/meter.c
@@ -0,0 +1,604 @@
+/*
+ * Copyright (c) 2017 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
+ * License as published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/if.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/kernel.h>
+#include <linux/openvswitch.h>
+#include <linux/netlink.h>
+#include <linux/rculist.h>
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "datapath.h"
+#include "meter.h"
+
+#define METER_HASH_BUCKETS 1024
+
+static const struct nla_policy meter_policy[OVS_METER_ATTR_MAX + 1] = {
+ [OVS_METER_ATTR_ID] = { .type = NLA_U32, },
+ [OVS_METER_ATTR_KBPS] = { .type = NLA_FLAG },
+ [OVS_METER_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
+ [OVS_METER_ATTR_BANDS] = { .type = NLA_NESTED },
+ [OVS_METER_ATTR_USED] = { .type = NLA_U64 },
+ [OVS_METER_ATTR_CLEAR] = { .type = NLA_FLAG },
+ [OVS_METER_ATTR_MAX_METERS] = { .type = NLA_U32 },
+ [OVS_METER_ATTR_MAX_BANDS] = { .type = NLA_U32 },
+};
+
+static const struct nla_policy band_policy[OVS_BAND_ATTR_MAX + 1] = {
+ [OVS_BAND_ATTR_TYPE] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_RATE] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_BURST] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
+};
+
+static void rcu_free_ovs_meter_callback(struct rcu_head *rcu)
+{
+ struct dp_meter *meter = container_of(rcu, struct dp_meter, rcu);
+
+ kfree(meter);
+}
+
+static void ovs_meter_free(struct dp_meter *meter)
+{
+ if (!meter)
+ return;
+
+ call_rcu(&meter->rcu, rcu_free_ovs_meter_callback);
+}
+
+static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
+ u32 meter_id)
+{
+ return &dp->meters[meter_id & (METER_HASH_BUCKETS - 1)];
+}
+
+/* Call with ovs_mutex or RCU read lock. */
+static struct dp_meter *lookup_meter(const struct datapath *dp,
+ u32 meter_id)
+{
+ struct dp_meter *meter;
+ struct hlist_head *head;
+
+ head = meter_hash_bucket(dp, meter_id);
+ hlist_for_each_entry_rcu(meter, head, dp_hash_node) {
+ if (meter->id == meter_id)
+ return meter;
+ }
+ return NULL;
+}
+
+static void attach_meter(struct datapath *dp, struct dp_meter *meter)
+{
+ struct hlist_head *head = meter_hash_bucket(dp, meter->id);
+
+ hlist_add_head_rcu(&meter->dp_hash_node, head);
+}
+
+static void detach_meter(struct dp_meter *meter)
+{
+ ASSERT_OVSL();
+ if (meter)
+ hlist_del_rcu(&meter->dp_hash_node);
+}
+
+static struct sk_buff *
+ovs_meter_cmd_reply_start(struct genl_info *info, u8 cmd,
+ struct ovs_header **ovs_reply_header)
+{
+ struct sk_buff *skb;
+ struct ovs_header *ovs_header = info->userhdr;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return ERR_PTR(-ENOMEM);
+
+ *ovs_reply_header = genlmsg_put(skb, info->snd_portid,
+ info->snd_seq,
+ &dp_meter_genl_family, 0, cmd);
+ if (!ovs_reply_header) {
+ nlmsg_free(skb);
+ return ERR_PTR(-EMSGSIZE);
+ }
+ (*ovs_reply_header)->dp_ifindex = ovs_header->dp_ifindex;
+
+ return skb;
+}
+
+static int ovs_meter_cmd_reply_stats(struct sk_buff *reply, u32 meter_id,
+ struct dp_meter *meter)
+{
+ struct nlattr *nla;
+ struct dp_meter_band *band;
+ u16 i;
+
+ if (nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id))
+ goto error;
+
+ if (!meter)
+ return 0;
+
+ if (nla_put(reply, OVS_METER_ATTR_STATS,
+ sizeof(struct ovs_flow_stats), &meter->stats) ||
+ nla_put_u64_64bit(reply, OVS_METER_ATTR_USED, meter->used,
+ OVS_METER_ATTR_PAD))
+ goto error;
+
+ nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
+ if (!nla)
+ goto error;
+
+ band = meter->bands;
+
+ for (i = 0; i < meter->n_bands; ++i, ++band) {
+ struct nlattr *band_nla;
+
+ band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
+ if (!band_nla || nla_put(reply, OVS_BAND_ATTR_STATS,
+ sizeof(struct ovs_flow_stats),
+ &band->stats))
+ goto error;
+ nla_nest_end(reply, band_nla);
+ }
+ nla_nest_end(reply, nla);
+
+ return 0;
+error:
+ return -EMSGSIZE;
+}
+
+static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
+{
+ struct sk_buff *reply;
+ struct ovs_header *ovs_reply_header;
+ struct nlattr *nla, *band_nla;
+ int err;
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
+ &ovs_reply_header);
+ if (!reply)
+ return PTR_ERR(reply);
+
+ if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
+ nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
+ goto nla_put_failure;
+
+ nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
+ if (!nla)
+ goto nla_put_failure;
+
+ band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
+ if (!band_nla)
+ goto nla_put_failure;
+ /* Currently only DROP band type is supported. */
+ if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
+ goto nla_put_failure;
+ nla_nest_end(reply, band_nla);
+ nla_nest_end(reply, nla);
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+nla_put_failure:
+ nlmsg_free(reply);
+ err = -EMSGSIZE;
+ return err;
+}
+
+static struct dp_meter *dp_meter_create(struct nlattr **a)
+{
+ struct nlattr *nla;
+ int rem;
+ u16 n_bands = 0;
+ struct dp_meter *meter;
+ struct dp_meter_band *band;
+ int err;
+
+ /* Validate attributes, count the bands. */
+ if (!a[OVS_METER_ATTR_BANDS])
+ return ERR_PTR(-EINVAL);
+
+ nla_for_each_nested(nla, a[OVS_METER_ATTR_BANDS], rem)
+ if (++n_bands > DP_MAX_BANDS)
+ return ERR_PTR(-EINVAL);
+
+ /* Allocate and set up the meter before locking anything. */
+ meter = kzalloc(n_bands * sizeof(struct dp_meter_band) +
+ sizeof(*meter), GFP_KERNEL);
+ if (!meter)
+ return ERR_PTR(-ENOMEM);
+
+ meter->used = ktime_get_ns() / 1000 / 1000;
+ meter->kbps = a[OVS_METER_ATTR_KBPS] ? 1 : 0;
+ meter->keep_stats = !a[OVS_METER_ATTR_CLEAR];
+ spin_lock_init(&meter->lock);
+ if (meter->keep_stats && a[OVS_METER_ATTR_STATS]) {
+ meter->stats = *(struct ovs_flow_stats *)
+ nla_data(a[OVS_METER_ATTR_STATS]);
+ }
+ meter->n_bands = n_bands;
+
+ /* Set up meter bands. */
+ band = meter->bands;
+ nla_for_each_nested(nla, a[OVS_METER_ATTR_BANDS], rem) {
+ struct nlattr *attr[OVS_BAND_ATTR_MAX + 1];
+ u32 band_max_delta_t;
+
+ err = nla_parse((struct nlattr **)&attr, OVS_BAND_ATTR_MAX,
+ nla_data(nla), nla_len(nla), band_policy,
+ NULL);
+ if (err)
+ goto exit_free_meter;
+
+ if (!attr[OVS_BAND_ATTR_TYPE] ||
+ !attr[OVS_BAND_ATTR_RATE] ||
+ !attr[OVS_BAND_ATTR_BURST]) {
+ err = -EINVAL;
+ goto exit_free_meter;
+ }
+
+ band->type = nla_get_u32(attr[OVS_BAND_ATTR_TYPE]);
+ band->rate = nla_get_u32(attr[OVS_BAND_ATTR_RATE]);
+ band->burst_size = nla_get_u32(attr[OVS_BAND_ATTR_BURST]);
+ /* Figure out max delta_t that is enough to fill any bucket.
+ * Keep max_delta_t size to the bucket units:
+ * pkts => 1/1000 packets, kilobits => bits.
+ */
+ band_max_delta_t = (band->burst_size + band->rate) * 1000;
+ /* Start with a full bucket. */
+ band->bucket = band_max_delta_t;
+ if (band_max_delta_t > meter->max_delta_t)
+ meter->max_delta_t = band_max_delta_t;
+ band++;
+ }
+
+ return meter;
+
+exit_free_meter:
+ kfree(meter);
+ return ERR_PTR(err);
+}
+
+static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ struct dp_meter *meter, *old_meter;
+ struct sk_buff *reply;
+ struct ovs_header *ovs_reply_header;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct datapath *dp;
+ int err;
+ u32 meter_id;
+ bool failed;
+
+ meter = dp_meter_create(a);
+ if (IS_ERR_OR_NULL(meter))
+ return PTR_ERR(meter);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
+ &ovs_reply_header);
+ if (IS_ERR(reply)) {
+ err = PTR_ERR(reply);
+ goto exit_free_meter;
+ }
+
+ ovs_lock();
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ if (!a[OVS_METER_ATTR_ID]) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ /* Cannot fail after this. */
+ old_meter = lookup_meter(dp, meter_id);
+ detach_meter(old_meter);
+ attach_meter(dp, meter);
+ ovs_unlock();
+
+ /* Build response with the meter_id and stats from
+ * the old meter, if any.
+ */
+ failed = nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id);
+ WARN_ON(failed);
+ if (old_meter) {
+ spin_lock_bh(&old_meter->lock);
+ if (old_meter->keep_stats) {
+ err = ovs_meter_cmd_reply_stats(reply, meter_id,
+ old_meter);
+ WARN_ON(err);
+ }
+ spin_unlock_bh(&old_meter->lock);
+ ovs_meter_free(old_meter);
+ }
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+exit_free_meter:
+ kfree(meter);
+ return err;
+}
+
+static int ovs_meter_cmd_get(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ u32 meter_id;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct ovs_header *ovs_reply_header;
+ struct datapath *dp;
+ int err;
+ struct sk_buff *reply;
+ struct dp_meter *meter;
+
+ if (!a[OVS_METER_ATTR_ID])
+ return -EINVAL;
+
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_GET,
+ &ovs_reply_header);
+ if (IS_ERR(reply))
+ return PTR_ERR(reply);
+
+ ovs_lock();
+
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ /* Locate meter, copy stats. */
+ meter = lookup_meter(dp, meter_id);
+ if (!meter) {
+ err = -ENOENT;
+ goto exit_unlock;
+ }
+
+ spin_lock_bh(&meter->lock);
+ err = ovs_meter_cmd_reply_stats(reply, meter_id, meter);
+ spin_unlock_bh(&meter->lock);
+ if (err)
+ goto exit_unlock;
+
+ ovs_unlock();
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+ return err;
+}
+
+static int ovs_meter_cmd_del(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ u32 meter_id;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct ovs_header *ovs_reply_header;
+ struct datapath *dp;
+ int err;
+ struct sk_buff *reply;
+ struct dp_meter *old_meter;
+
+ if (!a[OVS_METER_ATTR_ID])
+ return -EINVAL;
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_DEL,
+ &ovs_reply_header);
+ if (IS_ERR(reply))
+ return PTR_ERR(reply);
+
+ ovs_lock();
+
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ old_meter = lookup_meter(dp, meter_id);
+ if (old_meter) {
+ spin_lock_bh(&old_meter->lock);
+ err = ovs_meter_cmd_reply_stats(reply, meter_id, old_meter);
+ WARN_ON(err);
+ spin_unlock_bh(&old_meter->lock);
+ detach_meter(old_meter);
+ }
+ ovs_unlock();
+ ovs_meter_free(old_meter);
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+ return err;
+}
+
+/* Meter action execution.
+ *
+ * Return true 'meter_id' drop band is triggered. The 'skb' should be
+ * dropped by the caller'.
+ */
+bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_key *key, u32 meter_id)
+{
+ struct dp_meter *meter;
+ struct dp_meter_band *band;
+ long long int now_ms = ktime_get_ns() / 1000 / 1000;
+ long long int long_delta_ms;
+ u32 delta_ms;
+ u32 cost;
+ int i, band_exceeded_max = -1;
+ u32 band_exceeded_rate = 0;
+
+ meter = lookup_meter(dp, meter_id);
+ /* Do not drop the packet when there is no meter. */
+ if (!meter)
+ return false;
+
+ /* Lock the meter while using it. */
+ spin_lock(&meter->lock);
+
+ long_delta_ms = (now_ms - meter->used); /* ms */
+
+ /* Make sure delta_ms will not be too large, so that bucket will not
+ * wrap around below.
+ */
+ delta_ms = (long_delta_ms > (long long int)meter->max_delta_t)
+ ? meter->max_delta_t : (u32)long_delta_ms;
+
+ /* Update meter statistics.
+ */
+ meter->used = now_ms;
+ meter->stats.n_packets += 1;
+ meter->stats.n_bytes += skb->len;
+
+ /* Bucket rate is either in kilobits per second, or in packets per
+ * second. We maintain the bucket in the units of either bits or
+ * 1/1000th of a packet, correspondingly.
+ * Then, when rate is multiplied with milliseconds, we get the
+ * bucket units:
+ * msec * kbps = bits, and
+ * msec * packets/sec = 1/1000 packets.
+ *
+ * 'cost' is the number of bucket units in this packet.
+ */
+ cost = (meter->kbps) ? skb->len * 8 : 1000;
+
+ /* Update all bands and find the one hit with the highest rate. */
+ for (i = 0; i < meter->n_bands; ++i) {
+ long long int max_bucket_size;
+
+ band = &meter->bands[i];
+ max_bucket_size = (band->burst_size + band->rate) * 1000;
+
+ band->bucket += delta_ms * band->rate;
+ if (band->bucket > max_bucket_size)
+ band->bucket = max_bucket_size;
+
+ if (band->bucket >= cost) {
+ band->bucket -= cost;
+ } else if (band->rate > band_exceeded_rate) {
+ band_exceeded_rate = band->rate;
+ band_exceeded_max = i;
+ }
+ }
+
+ if (band_exceeded_max >= 0) {
+ /* Update band statistics. */
+ band = &meter->bands[band_exceeded_max];
+ band->stats.n_packets += 1;
+ band->stats.n_bytes += skb->len;
+
+ /* Drop band triggered, let the caller drop the 'skb'. */
+ if (band->type == OVS_METER_BAND_TYPE_DROP) {
+ spin_unlock(&meter->lock);
+ return true;
+ }
+ }
+
+ spin_unlock(&meter->lock);
+ return false;
+}
+
+static struct genl_ops dp_meter_genl_ops[] = {
+ { .cmd = OVS_METER_CMD_FEATURES,
+ .flags = 0, /* OK for unprivileged users. */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_features
+ },
+ { .cmd = OVS_METER_CMD_SET,
+ .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_set,
+ },
+ { .cmd = OVS_METER_CMD_GET,
+ .flags = 0, /* OK for unprivileged users. */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_get,
+ },
+ { .cmd = OVS_METER_CMD_DEL,
+ .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_del
+ },
+};
+
+static const struct genl_multicast_group ovs_meter_multicast_group = {
+ .name = OVS_METER_MCGROUP,
+};
+
+struct genl_family dp_meter_genl_family __ro_after_init = {
+ .hdrsize = sizeof(struct ovs_header),
+ .name = OVS_METER_FAMILY,
+ .version = OVS_METER_VERSION,
+ .maxattr = OVS_METER_ATTR_MAX,
+ .netnsok = true,
+ .parallel_ops = true,
+ .ops = dp_meter_genl_ops,
+ .n_ops = ARRAY_SIZE(dp_meter_genl_ops),
+ .mcgrps = &ovs_meter_multicast_group,
+ .n_mcgrps = 1,
+ .module = THIS_MODULE,
+};
+
+int ovs_meters_init(struct datapath *dp)
+{
+ int i;
+
+ dp->meters = kmalloc_array(METER_HASH_BUCKETS,
+ sizeof(struct hlist_head), GFP_KERNEL);
+
+ if (!dp->meters)
+ return -ENOMEM;
+
+ for (i = 0; i < METER_HASH_BUCKETS; i++)
+ INIT_HLIST_HEAD(&dp->meters[i]);
+
+ return 0;
+}
+
+void ovs_meters_exit(struct datapath *dp)
+{
+ int i;
+
+ for (i = 0; i < METER_HASH_BUCKETS; i++) {
+ struct hlist_head *head = &dp->meters[i];
+ struct dp_meter *meter;
+ struct hlist_node *n;
+
+ hlist_for_each_entry_safe(meter, n, head, dp_hash_node)
+ kfree(meter);
+ }
+
+ kfree(dp->meters);
+}
diff --git a/net/openvswitch/meter.h b/net/openvswitch/meter.h
new file mode 100644
index 000000000000..964ace2650f8
--- /dev/null
+++ b/net/openvswitch/meter.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2017 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
+ * License as published by the Free Software Foundation.
+ */
+
+#ifndef METER_H
+#define METER_H 1
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/netlink.h>
+#include <linux/openvswitch.h>
+#include <linux/genetlink.h>
+#include <linux/skbuff.h>
+
+#include "flow.h"
+struct datapath;
+
+#define DP_MAX_BANDS 1
+
+struct dp_meter_band {
+ u32 type;
+ u32 rate;
+ u32 burst_size;
+ u32 bucket; /* 1/1000 packets, or in bits */
+ struct ovs_flow_stats stats;
+};
+
+struct dp_meter {
+ spinlock_t lock; /* Per meter lock */
+ struct rcu_head rcu;
+ struct hlist_node dp_hash_node; /*Element in datapath->meters
+ * hash table.
+ */
+ u32 id;
+ u16 kbps:1, keep_stats:1;
+ u16 n_bands;
+ u32 max_delta_t;
+ u64 used;
+ struct ovs_flow_stats stats;
+ struct dp_meter_band bands[];
+};
+
+extern struct genl_family dp_meter_genl_family;
+int ovs_meters_init(struct datapath *dp);
+void ovs_meters_exit(struct datapath *dp);
+bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_key *key, u32 meter_id);
+
+#endif /* meter.h */
--
1.8.3.1
^ permalink raw reply related
* [net-next v2 2/4] openvswitch: export get_dp() API.
From: Andy Zhou @ 2017-10-17 7:36 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1508225805-11613-1-git-send-email-azhou@ovn.org>
Later patches will invoke get_dp() outside of datapath.c. Export it.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
net/openvswitch/datapath.c | 29 -----------------------------
net/openvswitch/datapath.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c3aec6227c91..ac7154018676 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -142,35 +142,6 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *,
const struct dp_upcall_info *,
uint32_t cutlen);
-/* Must be called with rcu_read_lock. */
-static struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
-{
- struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
-
- if (dev) {
- struct vport *vport = ovs_internal_dev_get_vport(dev);
- if (vport)
- return vport->dp;
- }
-
- return NULL;
-}
-
-/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
- * returned dp pointer valid.
- */
-static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
-{
- struct datapath *dp;
-
- WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
- rcu_read_lock();
- dp = get_dp_rcu(net, dp_ifindex);
- rcu_read_unlock();
-
- return dp;
-}
-
/* Must be called with rcu_read_lock or ovs_mutex. */
const char *ovs_dp_name(const struct datapath *dp)
{
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 480600649d0b..ad14b571219d 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -30,6 +30,7 @@
#include "conntrack.h"
#include "flow.h"
#include "flow_table.h"
+#include "vport-internal_dev.h"
#define DP_MAX_PORTS USHRT_MAX
#define DP_VPORT_HASH_BUCKETS 1024
@@ -190,6 +191,36 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
return ovs_lookup_vport(dp, port_no);
}
+/* Must be called with rcu_read_lock. */
+static inline struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
+{
+ struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
+
+ if (dev) {
+ struct vport *vport = ovs_internal_dev_get_vport(dev);
+
+ if (vport)
+ return vport->dp;
+ }
+
+ return NULL;
+}
+
+/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
+ * returned dp pointer valid.
+ */
+static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
+{
+ struct datapath *dp;
+
+ WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
+ rcu_read_lock();
+ dp = get_dp_rcu(net, dp_ifindex);
+ rcu_read_unlock();
+
+ return dp;
+}
+
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
--
1.8.3.1
^ 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