* [PATCH net-next 11/12] net: hns3: Fix speed/duplex information loss problem when executing ethtool ethx cmd of VF
From: Salil Mehta @ 2018-09-21 15:41 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, Fuyun Liang
In-Reply-To: <20180921154148.26756-1-salil.mehta@huawei.com>
From: Fuyun Liang <liangfuyun1@huawei.com>
Our VF has not implemented the ops for get_port_type. So when we executing
ethtool ethx cmd of VF, hns3_get_link_ksettings will return directly. And
we can not query anything.
To support get_link_ksettings for VF, this patch replaces get_port_type
with get_media_type. If the media type is HNAE3_MEDIA_TYPE_NONE,
hns3_get_link_ksettings will return link information of VF.
Fixes: 12f46bc1d447 ("net: hns3: Refine hns3_get_link_ksettings()")
Signed-off-by: Fuyun Liang <liangfuyun1@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 66 +++++++++++++---------
1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index 46701cf..8803a87 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -545,55 +545,67 @@ static int hns3_set_pauseparam(struct net_device *netdev,
return -EOPNOTSUPP;
}
+static void hns3_get_ksettings(struct hnae3_handle *h,
+ struct ethtool_link_ksettings *cmd)
+{
+ const struct hnae3_ae_ops *ops = h->ae_algo->ops;
+
+ /* 1.auto_neg & speed & duplex from cmd */
+ if (ops->get_ksettings_an_result)
+ ops->get_ksettings_an_result(h,
+ &cmd->base.autoneg,
+ &cmd->base.speed,
+ &cmd->base.duplex);
+
+ /* 2.get link mode*/
+ if (ops->get_link_mode)
+ ops->get_link_mode(h,
+ cmd->link_modes.supported,
+ cmd->link_modes.advertising);
+
+ /* 3.mdix_ctrl&mdix get from phy reg */
+ if (ops->get_mdix_mode)
+ ops->get_mdix_mode(h, &cmd->base.eth_tp_mdix_ctrl,
+ &cmd->base.eth_tp_mdix);
+}
+
static int hns3_get_link_ksettings(struct net_device *netdev,
struct ethtool_link_ksettings *cmd)
{
struct hnae3_handle *h = hns3_get_handle(netdev);
const struct hnae3_ae_ops *ops;
+ u8 media_type;
u8 link_stat;
if (!h->ae_algo || !h->ae_algo->ops)
return -EOPNOTSUPP;
ops = h->ae_algo->ops;
- if (ops->get_port_type)
- ops->get_port_type(h, &cmd->base.port);
+ if (ops->get_media_type)
+ ops->get_media_type(h, &media_type);
else
return -EOPNOTSUPP;
- switch (cmd->base.port) {
- case PORT_FIBRE:
- /* 1.auto_neg & speed & duplex from cmd */
- if (ops->get_ksettings_an_result)
- ops->get_ksettings_an_result(h,
- &cmd->base.autoneg,
- &cmd->base.speed,
- &cmd->base.duplex);
- else
- return -EOPNOTSUPP;
-
- /* 2.get link mode*/
- if (ops->get_link_mode)
- ops->get_link_mode(h,
- cmd->link_modes.supported,
- cmd->link_modes.advertising);
-
- /* 3.mdix_ctrl&mdix get from phy reg */
- if (ops->get_mdix_mode)
- ops->get_mdix_mode(h, &cmd->base.eth_tp_mdix_ctrl,
- &cmd->base.eth_tp_mdix);
-
+ switch (media_type) {
+ case HNAE3_MEDIA_TYPE_NONE:
+ cmd->base.port = PORT_NONE;
+ hns3_get_ksettings(h, cmd);
+ break;
+ case HNAE3_MEDIA_TYPE_FIBER:
+ cmd->base.port = PORT_FIBRE;
+ hns3_get_ksettings(h, cmd);
break;
- case PORT_TP:
+ case HNAE3_MEDIA_TYPE_COPPER:
if (!netdev->phydev)
return -EOPNOTSUPP;
+ cmd->base.port = PORT_TP;
phy_ethtool_ksettings_get(netdev->phydev, cmd);
break;
default:
- netdev_warn(netdev,
- "Unknown port type, neither Fibre/Copper detected");
+
+ netdev_warn(netdev, "Unknown media type");
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 12/12] net: hns3: Remove redundant hclge_get_port_type()
From: Salil Mehta @ 2018-09-21 15:41 UTC (permalink / raw)
To: davem
Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
linux-kernel, linuxarm, Fuyun Liang
In-Reply-To: <20180921154148.26756-1-salil.mehta@huawei.com>
From: Peng Li <lipeng321@huawei.com>
This patch removes hclge_get_port_type which is redundant.
Signed-off-by: Fuyun Liang <liangfuyun1@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 1 -
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 22 ----------------------
2 files changed, 23 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index fc87e1b..564afd4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -411,7 +411,6 @@ struct hnae3_ae_ops {
void (*get_link_mode)(struct hnae3_handle *handle,
unsigned long *supported,
unsigned long *advertising);
- void (*get_port_type)(struct hnae3_handle *handle, u8 *port_type);
};
struct hnae3_dcb_ops {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 73d9d0d..2de5a00 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5970,27 +5970,6 @@ static void hclge_get_link_mode(struct hnae3_handle *handle,
}
}
-static void hclge_get_port_type(struct hnae3_handle *handle,
- u8 *port_type)
-{
- struct hclge_vport *vport = hclge_get_vport(handle);
- struct hclge_dev *hdev = vport->back;
- u8 media_type = hdev->hw.mac.media_type;
-
- switch (media_type) {
- case HNAE3_MEDIA_TYPE_FIBER:
- *port_type = PORT_FIBRE;
- break;
- case HNAE3_MEDIA_TYPE_COPPER:
- *port_type = PORT_TP;
- break;
- case HNAE3_MEDIA_TYPE_UNKNOWN:
- default:
- *port_type = PORT_OTHER;
- break;
- }
-}
-
static const struct hnae3_ae_ops hclge_ops = {
.init_ae_dev = hclge_init_ae_dev,
.uninit_ae_dev = hclge_uninit_ae_dev,
@@ -6048,7 +6027,6 @@ static const struct hnae3_ae_ops hclge_ops = {
.get_regs = hclge_get_regs,
.set_led_id = hclge_set_led_id,
.get_link_mode = hclge_get_link_mode,
- .get_port_type = hclge_get_port_type,
};
static struct hnae3_ae_algo ae_algo = {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net] ixgbe: check return value of napi_complete_done()
From: Eric Dumazet @ 2018-09-21 15:43 UTC (permalink / raw)
To: Alexei Starovoitov, Eric Dumazet, Song Liu
Cc: Jeff Kirsher, netdev, intel-wired-lan@lists.osuosl.org,
Kernel Team, stable@vger.kernel.org, Alexei Starovoitov
In-Reply-To: <d15e9eec-6236-af26-0eb5-84e7dc27b4d7@fb.com>
On 09/21/2018 08:14 AM, Alexei Starovoitov wrote:
> On 9/21/18 7:59 AM, Eric Dumazet wrote:
>>
>>
>> On 09/21/2018 07:55 AM, Alexei Starovoitov wrote:
>>
>>>
>>> should we remove ndo_poll_controller then?
>>> My understanding that the patch helps by not letting
>>> drivers do napi_schedule() for all queues into this_cpu, right?
>>> But most of the drivers do exactly that in their ndo_poll_controller
>>> implementations. Means most of the drivers will experience
>>> this nasty behavior.
>>>
>>
>> Some legacy drivers do not use NAPI yet, but provide ndo_poll_controller()
>>
>> I believe users caring about system behavior with multi queue NIC are
>> all using NAPI enabled drivers, so this should be fine.
>
> I'm not following.
> All modern napi enabled drivers need to _remove_ ndo_poll_controller
> from the driver. This is a lot of churn.
netpoll could skip calling ndo_poll_controller() if at least one NAPI
has been registered on the device.
diff --git a/net/core/netpoll.c b/net/core/netpoll.c
index 57557a6a950cc9cdff959391576a03381d328c1a..149474c1ad71dde295d3a2b085ef51eb50278d81 100644
--- a/net/core/netpoll.c
+++ b/net/core/netpoll.c
@@ -189,7 +189,6 @@ static void poll_napi(struct net_device *dev)
static void netpoll_poll_dev(struct net_device *dev)
{
- const struct net_device_ops *ops;
struct netpoll_info *ni = rcu_dereference_bh(dev->npinfo);
/* Don't do any rx activity if the dev_lock mutex is held
@@ -204,14 +203,12 @@ static void netpoll_poll_dev(struct net_device *dev)
return;
}
- ops = dev->netdev_ops;
- if (!ops->ndo_poll_controller) {
- up(&ni->dev_lock);
- return;
- }
+ if (list_empty(&dev->napi_list)) {
+ const struct net_device_ops *ops = dev->netdev_ops;
- /* Process pending work on NIC */
- ops->ndo_poll_controller(dev);
+ if (ops->ndo_poll_controller)
+ ops->ndo_poll_controller(dev);
+ }
poll_napi(dev);
But this looks a bit risky, I know some drivers use NAPI only for RX,
and conventional hard IRQ handler for TX completions.
Better be safe, and apply a small patch series, I can handle that, do not worry.
> Isn't it cleaner (less error prone) to introduce new ndo
> for legacy drivers without napi?
Not really, this is basically not doable, since no one of us can test this.
^ permalink raw reply related
* [PATCH ipsec] net: xfrm: pass constant family to nf_hook function
From: Florian Westphal @ 2018-09-21 10:35 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Florian Westphal
Unfortunately some versions of gcc emit following warning:
linux/compiler.h:252:20: warning: array subscript is above array bounds [-Warray-bounds]
hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
^~~~~~~~~~~~~~~~~~~~~
xfrm_output_resume passes non-const 'pf' argument so compiler can't know
that the affected statement above will never be executed (we only
pass either NFPROTO_IPV4 or NFPROTO_IPV6), this change makes this
explicit.
Another solution would be to increase hooks_arp[] size, but that
increases struct net size needlesly.
Reported-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
David, i hope this will silence the warning, would be nice
if you could test it.
I don't really like this patch, but I see no better solution
expect needless increase of hooks_arp[].
Any other idea?
net/xfrm/xfrm_output.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 45ba07ab3e4f..199c0e782ac7 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -152,11 +152,24 @@ int xfrm_output_resume(struct sk_buff *skb, int err)
if (!skb_dst(skb)->xfrm)
return dst_output(net, skb->sk, skb);
- err = nf_hook(skb_dst(skb)->ops->family,
- NF_INET_POST_ROUTING, net, skb->sk, skb,
- NULL, skb_dst(skb)->dev, xfrm_output2);
- if (unlikely(err != 1))
- goto out;
+ switch (skb_dst(skb)->ops->family) {
+ case AF_INET:
+ err = nf_hook(NFPROTO_IPV4,
+ NF_INET_POST_ROUTING, net, skb->sk, skb,
+ NULL, skb_dst(skb)->dev, xfrm_output2);
+ if (unlikely(err != 1))
+ goto out;
+ break;
+ case AF_INET6:
+ err = nf_hook(NFPROTO_IPV6,
+ NF_INET_POST_ROUTING, net, skb->sk, skb,
+ NULL, skb_dst(skb)->dev, xfrm_output2);
+ if (unlikely(err != 1))
+ goto out;
+ break;
+ default:
+ break;
+ }
}
if (err == -EINPROGRESS)
--
2.16.4
^ permalink raw reply related
* [PATCH 2/2] net: if_arp: use define instead of hard-coded value
From: Håkon Bugge @ 2018-09-21 10:39 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: linux-kernel, netdev
In-Reply-To: <20180921103930.1420679-1-Haakon.Bugge@oracle.com>
uapi/linux/if_arp.h includes linux/netdevice.h, which uses
IFNAMSIZ. Hence, use it instead of hard-coded value.
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
---
include/uapi/linux/if_arp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
index b68b4b3d9172..c3cc5a9e5eaf 100644
--- a/include/uapi/linux/if_arp.h
+++ b/include/uapi/linux/if_arp.h
@@ -118,7 +118,7 @@ struct arpreq {
struct sockaddr arp_ha; /* hardware address */
int arp_flags; /* flags */
struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
- char arp_dev[16];
+ char arp_dev[IFNAMSIZ];
};
struct arpreq_old {
--
2.14.3
^ permalink raw reply related
* [PATCH 1/2] net: if_arp: Fix incorrect indents
From: Håkon Bugge @ 2018-09-21 10:39 UTC (permalink / raw)
To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: linux-kernel, netdev
In-Reply-To: <20180921103930.1420679-1-Haakon.Bugge@oracle.com>
Fixing incorrect indents and align comments.
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
---
include/uapi/linux/if_arp.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
index 4605527ca41b..b68b4b3d9172 100644
--- a/include/uapi/linux/if_arp.h
+++ b/include/uapi/linux/if_arp.h
@@ -114,18 +114,18 @@
/* ARP ioctl request. */
struct arpreq {
- struct sockaddr arp_pa; /* protocol address */
- struct sockaddr arp_ha; /* hardware address */
- int arp_flags; /* flags */
- struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
- char arp_dev[16];
+ struct sockaddr arp_pa; /* protocol address */
+ struct sockaddr arp_ha; /* hardware address */
+ int arp_flags; /* flags */
+ struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
+ char arp_dev[16];
};
struct arpreq_old {
- struct sockaddr arp_pa; /* protocol address */
- struct sockaddr arp_ha; /* hardware address */
- int arp_flags; /* flags */
- struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
+ struct sockaddr arp_pa; /* protocol address */
+ struct sockaddr arp_ha; /* hardware address */
+ int arp_flags; /* flags */
+ struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
};
/* ARP Flag values. */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net] net: diag: Fix swapped src/dst in udp_dump_one.
From: Lorenzo Colitti @ 2018-09-21 10:46 UTC (permalink / raw)
To: David Miller
Cc: netdev, Maciej Żenczykowski, David Ahern,
Jeffrey Vander Stoep
In-Reply-To: <20180914.083038.452831271321088318.davem@davemloft.net>
On Fri, Sep 14, 2018 at 11:30 PM David Miller <davem@davemloft.net> wrote:
> Unfortunately I think we are stuck with how things are now.
>
> Indisputably, your patch breaks userland components that have
> workarounds in order to work with existing kernels. [...]
> I cannot apply this, sorry.
Understood. We're about to start using this UDP codepath on Android,
so I wanted to see if this would be fixed before we commit to the
current behaviour in our conformance tests, at which point we won't
ever be able to change it either.
Would you take a patch to add a one-line comment saying that this is
the way it is for backwards compatibility? If that comment were there
anyone else who finds this will not spend time debugging it and
immediately know what's going on. The fact that the ports are inverted
is not easy to spot on casual inspection.
^ permalink raw reply
* Re: [PATCH net-next] cxgb4vf: Add ethtool private flags for changing force_link_up
From: Arjun Vynipadath @ 2018-09-21 10:46 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, dt, nirranjan, indranil, Casey Leedom,
Ganesh Goudar
In-Reply-To: <20180918113914.58811ecf@cakuba.netronome.com>
On Tuesday, September 09/18/18, 2018 at 11:39:14 -0700, Jakub Kicinski wrote:
> On Tue, 18 Sep 2018 18:37:23 +0530, Arjun Vynipadath wrote:
> > Forcing link up of virtual interfaces even when physical link is down
> > causes packet drops and ping failures during bonding failover. Hence
> > adding a ethtool private flag to toggle force_link_up whenever required.
> >
> > Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
> > Signed-off-by: Casey Leedom <leedom@chelsio.com>
> > Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
>
> Could you describe how this mechanism relates to the existing
> ndo_set_vf_link_state, which you seem to not make use of:
>
> $ git grep ndo_set_vf_link_state -- drivers/net/ethernet/chelsio/
> $
>
> I understand you're configuring the setting from the VF side, but the
> question, as always, is: why ;)
Hi Jakub,
ndo_set_vf_link_state can't be presently used in our case.
We dont have firmware support to communicate the link flags set through
ndo_set_vf_link_state from pf (cxgb4) driver to vf (cxgb4vf) driver.
Thanks,
Arjun.
^ permalink raw reply
* [PATCH net-next V2 0/2] net/sched: Add hardware specific counters to TC actions
From: Eelco Chaudron @ 2018-09-21 11:13 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, xiyou.wangcong, jhs, jakub.kicinski
Add hardware specific counters to TC actions which will be exported
through the netlink API. This makes troubleshooting TC flower offload
easier, as it possible to differentiate the packets being offloaded.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
v2 - Rebased on latest net-next
Eelco Chaudron (2):
net/core: Add new basic hardware counter
net/sched: Add hardware specific counters to TC actions
include/net/act_api.h | 8 +++-
include/net/gen_stats.h | 4 ++
include/net/pkt_cls.h | 2 +
include/uapi/linux/gen_stats.h | 1 +
net/core/gen_stats.c | 73 ++++++++++++++++++++++++++++++----------
net/sched/act_api.c | 14 ++++++--
net/sched/act_gact.c | 6 +++
net/sched/act_mirred.c | 5 ++-
8 files changed, 85 insertions(+), 28 deletions(-)
^ permalink raw reply
* [PATCH net-next V2 1/2] net/core: Add new basic hardware counter
From: Eelco Chaudron @ 2018-09-21 11:13 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, xiyou.wangcong, jhs, jakub.kicinski
In-Reply-To: <20180921111330.3532.29886.stgit@wsfd-netdev20.ntdv.lab.eng.bos.redhat.com>
Add a new hardware specific basic counter, TCA_STATS_BASIC_HW. This can
be used to count packets/bytes processed by hardware offload.
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
include/net/gen_stats.h | 4 ++
include/uapi/linux/gen_stats.h | 1 +
net/core/gen_stats.c | 73 ++++++++++++++++++++++++++++++----------
3 files changed, 59 insertions(+), 19 deletions(-)
diff --git a/include/net/gen_stats.h b/include/net/gen_stats.h
index 883bb90..946bd53 100644
--- a/include/net/gen_stats.h
+++ b/include/net/gen_stats.h
@@ -44,6 +44,10 @@ void __gnet_stats_copy_basic(const seqcount_t *running,
struct gnet_stats_basic_packed *bstats,
struct gnet_stats_basic_cpu __percpu *cpu,
struct gnet_stats_basic_packed *b);
+int gnet_stats_copy_basic_hw(const seqcount_t *running,
+ struct gnet_dump *d,
+ struct gnet_stats_basic_cpu __percpu *cpu,
+ struct gnet_stats_basic_packed *b);
int gnet_stats_copy_rate_est(struct gnet_dump *d,
struct net_rate_estimator __rcu **ptr);
int gnet_stats_copy_queue(struct gnet_dump *d,
diff --git a/include/uapi/linux/gen_stats.h b/include/uapi/linux/gen_stats.h
index 24a861c..065408e 100644
--- a/include/uapi/linux/gen_stats.h
+++ b/include/uapi/linux/gen_stats.h
@@ -12,6 +12,7 @@ enum {
TCA_STATS_APP,
TCA_STATS_RATE_EST64,
TCA_STATS_PAD,
+ TCA_STATS_BASIC_HW,
__TCA_STATS_MAX,
};
#define TCA_STATS_MAX (__TCA_STATS_MAX - 1)
diff --git a/net/core/gen_stats.c b/net/core/gen_stats.c
index 188d693..65a2e82 100644
--- a/net/core/gen_stats.c
+++ b/net/core/gen_stats.c
@@ -162,30 +162,18 @@
}
EXPORT_SYMBOL(__gnet_stats_copy_basic);
-/**
- * gnet_stats_copy_basic - copy basic statistics into statistic TLV
- * @running: seqcount_t pointer
- * @d: dumping handle
- * @cpu: copy statistic per cpu
- * @b: basic statistics
- *
- * Appends the basic statistics to the top level TLV created by
- * gnet_stats_start_copy().
- *
- * Returns 0 on success or -1 with the statistic lock released
- * if the room in the socket buffer was not sufficient.
- */
int
-gnet_stats_copy_basic(const seqcount_t *running,
- struct gnet_dump *d,
- struct gnet_stats_basic_cpu __percpu *cpu,
- struct gnet_stats_basic_packed *b)
+___gnet_stats_copy_basic(const seqcount_t *running,
+ struct gnet_dump *d,
+ struct gnet_stats_basic_cpu __percpu *cpu,
+ struct gnet_stats_basic_packed *b,
+ int type)
{
struct gnet_stats_basic_packed bstats = {0};
__gnet_stats_copy_basic(running, &bstats, cpu, b);
- if (d->compat_tc_stats) {
+ if (d->compat_tc_stats && type == TCA_STATS_BASIC) {
d->tc_stats.bytes = bstats.bytes;
d->tc_stats.packets = bstats.packets;
}
@@ -196,14 +184,61 @@
memset(&sb, 0, sizeof(sb));
sb.bytes = bstats.bytes;
sb.packets = bstats.packets;
- return gnet_stats_copy(d, TCA_STATS_BASIC, &sb, sizeof(sb),
+ return gnet_stats_copy(d, type, &sb, sizeof(sb),
TCA_STATS_PAD);
}
return 0;
}
+
+/**
+ * gnet_stats_copy_basic - copy basic statistics into statistic TLV
+ * @running: seqcount_t pointer
+ * @d: dumping handle
+ * @cpu: copy statistic per cpu
+ * @b: basic statistics
+ *
+ * Appends the basic statistics to the top level TLV created by
+ * gnet_stats_start_copy().
+ *
+ * Returns 0 on success or -1 with the statistic lock released
+ * if the room in the socket buffer was not sufficient.
+ */
+int
+gnet_stats_copy_basic(const seqcount_t *running,
+ struct gnet_dump *d,
+ struct gnet_stats_basic_cpu __percpu *cpu,
+ struct gnet_stats_basic_packed *b)
+{
+ return ___gnet_stats_copy_basic(running, d, cpu, b,
+ TCA_STATS_BASIC);
+}
EXPORT_SYMBOL(gnet_stats_copy_basic);
/**
+ * gnet_stats_copy_basic_hw - copy basic hw statistics into statistic TLV
+ * @running: seqcount_t pointer
+ * @d: dumping handle
+ * @cpu: copy statistic per cpu
+ * @b: basic statistics
+ *
+ * Appends the basic statistics to the top level TLV created by
+ * gnet_stats_start_copy().
+ *
+ * Returns 0 on success or -1 with the statistic lock released
+ * if the room in the socket buffer was not sufficient.
+ */
+int
+gnet_stats_copy_basic_hw(const seqcount_t *running,
+ struct gnet_dump *d,
+ struct gnet_stats_basic_cpu __percpu *cpu,
+ struct gnet_stats_basic_packed *b)
+{
+ return ___gnet_stats_copy_basic(running, d, cpu, b,
+ TCA_STATS_BASIC_HW);
+}
+EXPORT_SYMBOL(gnet_stats_copy_basic_hw);
+
+/**
* gnet_stats_copy_rate_est - copy rate estimator statistics into statistics TLV
* @d: dumping handle
* @rate_est: rate estimator
^ permalink raw reply related
* [PATCH net-next V2 2/2] net/sched: Add hardware specific counters to TC actions
From: Eelco Chaudron @ 2018-09-21 11:14 UTC (permalink / raw)
To: netdev; +Cc: davem, jiri, xiyou.wangcong, jhs, jakub.kicinski
In-Reply-To: <20180921111330.3532.29886.stgit@wsfd-netdev20.ntdv.lab.eng.bos.redhat.com>
Add additional counters that will store the bytes/packets processed by
hardware. These will be exported through the netlink interface for
displaying by the iproute2 tc tool
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
include/net/act_api.h | 8 +++++---
include/net/pkt_cls.h | 2 +-
net/sched/act_api.c | 14 +++++++++++---
net/sched/act_gact.c | 6 +++++-
net/sched/act_mirred.c | 5 ++++-
5 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index c6f195b..1ddff33 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -31,10 +31,12 @@ struct tc_action {
int tcfa_action;
struct tcf_t tcfa_tm;
struct gnet_stats_basic_packed tcfa_bstats;
+ struct gnet_stats_basic_packed tcfa_bstats_hw;
struct gnet_stats_queue tcfa_qstats;
struct net_rate_estimator __rcu *tcfa_rate_est;
spinlock_t tcfa_lock;
struct gnet_stats_basic_cpu __percpu *cpu_bstats;
+ struct gnet_stats_basic_cpu __percpu *cpu_bstats_hw;
struct gnet_stats_queue __percpu *cpu_qstats;
struct tc_cookie __rcu *act_cookie;
struct tcf_chain *goto_chain;
@@ -94,7 +96,7 @@ struct tc_action_ops {
struct netlink_callback *, int,
const struct tc_action_ops *,
struct netlink_ext_ack *);
- void (*stats_update)(struct tc_action *, u64, u32, u64);
+ void (*stats_update)(struct tc_action *, u64, u32, u64, bool);
size_t (*get_fill_size)(const struct tc_action *act);
struct net_device *(*get_dev)(const struct tc_action *a);
void (*put_dev)(struct net_device *dev);
@@ -182,13 +184,13 @@ int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[], int bind,
#endif /* CONFIG_NET_CLS_ACT */
static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,
- u64 packets, u64 lastuse)
+ u64 packets, u64 lastuse, bool hw)
{
#ifdef CONFIG_NET_CLS_ACT
if (!a->ops->stats_update)
return;
- a->ops->stats_update(a, bytes, packets, lastuse);
+ a->ops->stats_update(a, bytes, packets, lastuse, hw);
#endif
}
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 75a3f3f..bbfe27f 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -318,7 +318,7 @@ static inline void tcf_exts_put_net(struct tcf_exts *exts)
for (i = 0; i < exts->nr_actions; i++) {
struct tc_action *a = exts->actions[i];
- tcf_action_stats_update(a, bytes, packets, lastuse);
+ tcf_action_stats_update(a, bytes, packets, lastuse, true);
}
preempt_enable();
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f118d6..1acd3a8 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -81,6 +81,7 @@ static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
static void free_tcf(struct tc_action *p)
{
free_percpu(p->cpu_bstats);
+ free_percpu(p->cpu_bstats_hw);
free_percpu(p->cpu_qstats);
tcf_set_action_cookie(&p->act_cookie, NULL);
@@ -372,9 +373,12 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
p->cpu_bstats = netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
if (!p->cpu_bstats)
goto err1;
+ p->cpu_bstats_hw = netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
+ if (!p->cpu_bstats_hw)
+ goto err2;
p->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
if (!p->cpu_qstats)
- goto err2;
+ goto err3;
}
spin_lock_init(&p->tcfa_lock);
p->tcfa_index = index;
@@ -386,15 +390,17 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
&p->tcfa_rate_est,
&p->tcfa_lock, NULL, est);
if (err)
- goto err3;
+ goto err4;
}
p->idrinfo = idrinfo;
p->ops = ops;
*a = p;
return 0;
-err3:
+err4:
free_percpu(p->cpu_qstats);
+err3:
+ free_percpu(p->cpu_bstats_hw);
err2:
free_percpu(p->cpu_bstats);
err1:
@@ -979,6 +985,8 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,
goto errout;
if (gnet_stats_copy_basic(NULL, &d, p->cpu_bstats, &p->tcfa_bstats) < 0 ||
+ gnet_stats_copy_basic_hw(NULL, &d, p->cpu_bstats_hw,
+ &p->tcfa_bstats_hw) < 0 ||
gnet_stats_copy_rate_est(&d, &p->tcfa_rate_est) < 0 ||
gnet_stats_copy_queue(&d, p->cpu_qstats,
&p->tcfa_qstats,
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index aa44d14..c89a7fa 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -157,7 +157,7 @@ static int tcf_gact_act(struct sk_buff *skb, const struct tc_action *a,
}
static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse)
+ u64 lastuse, bool hw)
{
struct tcf_gact *gact = to_gact(a);
int action = READ_ONCE(gact->tcf_action);
@@ -168,6 +168,10 @@ static void tcf_gact_stats_update(struct tc_action *a, u64 bytes, u32 packets,
if (action == TC_ACT_SHOT)
this_cpu_ptr(gact->common.cpu_qstats)->drops += packets;
+ if (hw)
+ _bstats_cpu_update(this_cpu_ptr(gact->common.cpu_bstats_hw),
+ bytes, packets);
+
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index a9d64bf..1dae5f2 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -283,12 +283,15 @@ static int tcf_mirred_act(struct sk_buff *skb, const struct tc_action *a,
}
static void tcf_stats_update(struct tc_action *a, u64 bytes, u32 packets,
- u64 lastuse)
+ u64 lastuse, bool hw)
{
struct tcf_mirred *m = to_mirred(a);
struct tcf_t *tm = &m->tcf_tm;
_bstats_cpu_update(this_cpu_ptr(a->cpu_bstats), bytes, packets);
+ if (hw)
+ _bstats_cpu_update(this_cpu_ptr(a->cpu_bstats_hw),
+ bytes, packets);
tm->lastuse = max_t(u64, tm->lastuse, lastuse);
}
^ permalink raw reply related
* Re: [PATCH 0/2] net/sched: Add hardware specific counters to TC actions
From: Eelco Chaudron @ 2018-09-21 11:14 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Miller, netdev, jhs, xiyou.wangcong, jiri, simon.horman,
Marcelo Ricardo Leitner, louis.peens, Paolo, Davide Caratti
In-Reply-To: <20180920071415.6ff73da6@cakuba.netronome.com>
On 20 Sep 2018, at 16:14, Jakub Kicinski wrote:
> On Thu, 20 Sep 2018 09:14:08 +0200, Eelco Chaudron wrote:
>> Is there anything else blocking from getting this into net-next?
>>
>> I still think this patch is beneficial for the full user experience, and
>> I’ve got requests from QA and others for this.
>
> Not from my perspective, the numbers look okay, feel free to repost!
Thanks, I sent out a v2 re-based on the latest net-next.
^ permalink raw reply
* Re: Regression caused by commit 7bb05b85bc2d ("r8169: don't use MSI-X on RTL8106e")
From: Andy Shevchenko @ 2018-09-21 17:08 UTC (permalink / raw)
To: Jian-Hong Pan
Cc: Kai-Heng Feng, Heiner Kallweit, Thomas Gleixner, netdev,
Linux Kernel Mailing List, Linux Upstreaming Team, Daniel Drake,
steved424
In-Reply-To: <CAPpJ_edB_G5QPj-by9Y6Zjj36Xm9dmKXy8Kv5gXMf4YfybvuwA@mail.gmail.com>
On Thu, Sep 13, 2018 at 8:53 AM Jian-Hong Pan <jian-hong@endlessm.com> wrote:
>
> 2018-09-12 16:19 GMT+08:00 Kai-Heng Feng <kai.heng.feng@canonical.com>:
> > at 14:32, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> >> On Wed, 12 Sep 2018, Kai-Heng Feng wrote:
> >>
> >>> There's a Dell machine with RTL8106e stops to work after S3 since the
> >>> commit introduced. So I am wondering if it's possible to revert the
> >>> commit and use DMI/subsystem id based quirk table?
> >>
> >>
> >> Probably.
Have you seen this thread:
https://patchwork.ozlabs.org/cover/968924/
and this one:
https://patchwork.kernel.org/patch/10583229/
?
> >
> >
> > Hopefully Jian-Hong can cook up a quirk table for the issue.
>
> Module r8169 gets nothing in the PCI BAR after system resumes which
> makes MSI-X fail on some ASUS laptops equipped with RTL8106e chip.
> https://www.spinics.net/lists/linux-pci/msg75598.html
>
> Actually, I am waiting for the patch "PCI: Reprogram bridge prefetch
> registers on resume" being merged.
> https://marc.info/?l=linux-pm&m=153680987814299&w=2
>
> It resolves the drivers which get nothing in PCI BAR after system resumes.
>
> After that, I can remove the falling back code of RTL8106e.
>
> Heiner, any comment?
>
> Regards,
> Jian-Hong Pan
>
> >>
> >>> It's because of commit bc976233a872 ("genirq/msi, x86/vector: Prevent
> >>> reservation mode for non maskable MSI") cleared the reservation mode, and
> >>> I
> >>> can see this after S3:
> >>>
> >>> [ 94.872838] do_IRQ: 3.33 No irq handler for vector
> >>
> >>
> >> It's not because of that commit, really. There is a interrupt sent after
> >> resume to the wrong vector for whatever reason. The MSI vector cannot be
> >> masked it seems in the device, but the driver should quiescen the device
> >> to
> >> a point where it does not send interrupts.
> >
> >
> > Understood.
> >
> >>
> >>> If the device uses MSI-X instead of MSI, the issue doesn't happen because
> >>> of
> >>> reservation mode.
> >>
> >>
> >> Reservation mode has absolutely nothing to do with that. What prevents the
> >> issue is the fact that MSI-X can be masked by the IRQ core.
> >
> >
> > So in this case I think keep the device using MSI-X is a better route, it's
> > MSI-X capable anyway.
> >
> >>
> >>> Is it something should be handled by x86 BIOS? Because I don't see this
> >>> issue
> >>> when I use Suspend-to-Idle, which doesn't use BIOS to do suspend.
> >>
> >>
> >> Suspend to idle works completely different and I don't see the BIOS at
> >> fault here. it's more an issue of MSI not being maskable on that device,
> >> which can't be fixed in BIOS or it's some half quiescened state which is
> >> used when suspending and that's a pure driver issue.
> >
> >
> > Understood.
> > Thanks for all the info!
> >
> > Kai-Heng
> >
> >>
> >> Thanks,
> >>
> >> tglx
> >
> >
> >
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH bpf-next 2/9] bpf: rework cgroup storage pointer passing
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
To simplify the following introduction of per-cpu cgroup storage,
let's rework a bit a mechanism of passing a pointer to a cgroup
storage into the bpf_get_local_storage(). Let's save a pointer
to the corresponding bpf_cgroup_storage structure, instead of
a pointer to the actual buffer.
It will help us to handle per-cpu storage later, which has
a different way of accessing to the actual data.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf-cgroup.h | 13 ++++---------
kernel/bpf/helpers.c | 8 ++++++--
kernel/bpf/local_storage.c | 3 ++-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index e9871b012dac..7e0c9a1d48b7 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -23,7 +23,8 @@ struct bpf_cgroup_storage;
extern struct static_key_false cgroup_bpf_enabled_key;
#define cgroup_bpf_enabled static_branch_unlikely(&cgroup_bpf_enabled_key)
-DECLARE_PER_CPU(void*, bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
+DECLARE_PER_CPU(struct bpf_cgroup_storage*,
+ bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
#define for_each_cgroup_storage_type(stype) \
for (stype = 0; stype < MAX_BPF_CGROUP_STORAGE_TYPE; stype++)
@@ -115,15 +116,9 @@ static inline void bpf_cgroup_storage_set(struct bpf_cgroup_storage
*storage[MAX_BPF_CGROUP_STORAGE_TYPE])
{
enum bpf_cgroup_storage_type stype;
- struct bpf_storage_buffer *buf;
-
- for_each_cgroup_storage_type(stype) {
- if (!storage[stype])
- continue;
- buf = READ_ONCE(storage[stype]->buf);
- this_cpu_write(bpf_cgroup_storage[stype], &buf->data[0]);
- }
+ for_each_cgroup_storage_type(stype)
+ this_cpu_write(bpf_cgroup_storage[stype], storage[stype]);
}
struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(struct bpf_prog *prog,
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 9070b2ace6aa..e42f8789b7ea 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -195,7 +195,8 @@ const struct bpf_func_proto bpf_get_current_cgroup_id_proto = {
};
#ifdef CONFIG_CGROUP_BPF
-DECLARE_PER_CPU(void*, bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
+DECLARE_PER_CPU(struct bpf_cgroup_storage*,
+ bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
{
@@ -204,8 +205,11 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
* verifier checks that its value is correct.
*/
enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
+ struct bpf_cgroup_storage *storage;
- return (unsigned long) this_cpu_read(bpf_cgroup_storage[stype]);
+ storage = this_cpu_read(bpf_cgroup_storage[stype]);
+
+ return (unsigned long)&READ_ONCE(storage->buf)->data[0];
}
const struct bpf_func_proto bpf_get_local_storage_proto = {
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 0bd9f19fc557..6742292fb39e 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -7,7 +7,8 @@
#include <linux/rbtree.h>
#include <linux/slab.h>
-DEFINE_PER_CPU(void*, bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
+DEFINE_PER_CPU(struct bpf_cgroup_storage*,
+ bpf_cgroup_storage[MAX_BPF_CGROUP_STORAGE_TYPE]);
#ifdef CONFIG_CGROUP_BPF
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 3/9] bpf: introduce per-cpu cgroup local storage
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
This commit introduced per-cpu cgroup local storage.
Per-cpu cgroup local storage is very similar to simple cgroup storage
(let's call it shared), except all the data is per-cpu.
The main goal of per-cpu variant is to implement super fast
counters (e.g. packet counters), which don't require neither
lookups, neither atomic operations.
>From userspace's point of view, accessing a per-cpu cgroup storage
is similar to other per-cpu map types (e.g. per-cpu hashmaps and
arrays).
Writing to a per-cpu cgroup storage is not atomic, but is performed
by copying longs, so some minimal atomicity is here, exactly
as with other per-cpu maps.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
include/linux/bpf-cgroup.h | 20 ++++-
include/linux/bpf.h | 1 +
include/linux/bpf_types.h | 1 +
include/uapi/linux/bpf.h | 1 +
kernel/bpf/helpers.c | 8 +-
kernel/bpf/local_storage.c | 148 ++++++++++++++++++++++++++++++++-----
kernel/bpf/syscall.c | 11 ++-
kernel/bpf/verifier.c | 15 +++-
8 files changed, 177 insertions(+), 28 deletions(-)
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 7e0c9a1d48b7..9bd907657f9b 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -37,7 +37,10 @@ struct bpf_storage_buffer {
};
struct bpf_cgroup_storage {
- struct bpf_storage_buffer *buf;
+ union {
+ struct bpf_storage_buffer *buf;
+ char __percpu *percpu_buf;
+ };
struct bpf_cgroup_storage_map *map;
struct bpf_cgroup_storage_key key;
struct list_head list;
@@ -109,6 +112,9 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
static inline enum bpf_cgroup_storage_type cgroup_storage_type(
struct bpf_map *map)
{
+ if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
+ return BPF_CGROUP_STORAGE_PERCPU;
+
return BPF_CGROUP_STORAGE_SHARED;
}
@@ -131,6 +137,10 @@ void bpf_cgroup_storage_unlink(struct bpf_cgroup_storage *storage);
int bpf_cgroup_storage_assign(struct bpf_prog *prog, struct bpf_map *map);
void bpf_cgroup_storage_release(struct bpf_prog *prog, struct bpf_map *map);
+int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key, void *value);
+int bpf_percpu_cgroup_storage_update(struct bpf_map *map, void *key,
+ void *value, u64 flags);
+
/* Wrappers for __cgroup_bpf_run_filter_skb() guarded by cgroup_bpf_enabled. */
#define BPF_CGROUP_RUN_PROG_INET_INGRESS(sk, skb) \
({ \
@@ -285,6 +295,14 @@ static inline struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(
struct bpf_prog *prog, enum bpf_cgroup_storage_type stype) { return 0; }
static inline void bpf_cgroup_storage_free(
struct bpf_cgroup_storage *storage) {}
+static inline int bpf_percpu_cgroup_storage_copy(struct bpf_map *map, void *key,
+ void *value) {
+ return 0;
+}
+static inline int bpf_percpu_cgroup_storage_update(struct bpf_map *map,
+ void *key, void *value, u64 flags) {
+ return 0;
+}
#define cgroup_bpf_enabled (0)
#define BPF_CGROUP_PRE_CONNECT_ENABLED(sk) (0)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index b457fbe7b70b..018299a595c8 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -274,6 +274,7 @@ struct bpf_prog_offload {
enum bpf_cgroup_storage_type {
BPF_CGROUP_STORAGE_SHARED,
+ BPF_CGROUP_STORAGE_PERCPU,
__BPF_CGROUP_STORAGE_MAX
};
diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index c9bd6fb765b0..5432f4c9f50e 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -43,6 +43,7 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_ARRAY, cgroup_array_map_ops)
#endif
#ifdef CONFIG_CGROUP_BPF
BPF_MAP_TYPE(BPF_MAP_TYPE_CGROUP_STORAGE, cgroup_storage_map_ops)
+BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE, cgroup_storage_map_ops)
#endif
BPF_MAP_TYPE(BPF_MAP_TYPE_HASH, htab_map_ops)
BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_HASH, htab_percpu_map_ops)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index aa5ccd2385ed..e2070d819e04 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -127,6 +127,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_SOCKHASH,
BPF_MAP_TYPE_CGROUP_STORAGE,
BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
+ BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
};
enum bpf_prog_type {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e42f8789b7ea..1f21ef1c4ad3 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -206,10 +206,16 @@ BPF_CALL_2(bpf_get_local_storage, struct bpf_map *, map, u64, flags)
*/
enum bpf_cgroup_storage_type stype = cgroup_storage_type(map);
struct bpf_cgroup_storage *storage;
+ void *ptr = NULL;
storage = this_cpu_read(bpf_cgroup_storage[stype]);
- return (unsigned long)&READ_ONCE(storage->buf)->data[0];
+ if (stype == BPF_CGROUP_STORAGE_SHARED)
+ ptr = &READ_ONCE(storage->buf)->data[0];
+ else
+ ptr = this_cpu_ptr(storage->percpu_buf);
+
+ return (unsigned long)ptr;
}
const struct bpf_func_proto bpf_get_local_storage_proto = {
diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
index 6742292fb39e..d991355b5b46 100644
--- a/kernel/bpf/local_storage.c
+++ b/kernel/bpf/local_storage.c
@@ -152,6 +152,71 @@ static int cgroup_storage_update_elem(struct bpf_map *map, void *_key,
return 0;
}
+int bpf_percpu_cgroup_storage_copy(struct bpf_map *_map, void *_key,
+ void *value)
+{
+ struct bpf_cgroup_storage_map *map = map_to_storage(_map);
+ struct bpf_cgroup_storage_key *key = _key;
+ struct bpf_cgroup_storage *storage;
+ int cpu, off = 0;
+ u32 size;
+
+ rcu_read_lock();
+ storage = cgroup_storage_lookup(map, key, false);
+ if (!storage) {
+ rcu_read_unlock();
+ return -ENOENT;
+ }
+
+ /* per_cpu areas are zero-filled and bpf programs can only
+ * access 'value_size' of them, so copying rounded areas
+ * will not leak any kernel data
+ */
+ size = round_up(_map->value_size, 8);
+ for_each_possible_cpu(cpu) {
+ bpf_long_memcpy(value + off,
+ per_cpu_ptr(storage->percpu_buf, cpu), size);
+ off += size;
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
+int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *_key,
+ void *value, u64 map_flags)
+{
+ struct bpf_cgroup_storage_map *map = map_to_storage(_map);
+ struct bpf_cgroup_storage_key *key = _key;
+ struct bpf_cgroup_storage *storage;
+ int cpu, off = 0;
+ u32 size;
+
+ if (unlikely(map_flags & BPF_EXIST))
+ return -EINVAL;
+
+ rcu_read_lock();
+ storage = cgroup_storage_lookup(map, key, false);
+ if (!storage) {
+ rcu_read_unlock();
+ return -ENOENT;
+ }
+
+ /* the user space will provide round_up(value_size, 8) bytes that
+ * will be copied into per-cpu area. bpf programs can only access
+ * value_size of it. During lookup the same extra bytes will be
+ * returned or zeros which were zero-filled by percpu_alloc,
+ * so no kernel data leaks possible
+ */
+ size = round_up(_map->value_size, 8);
+ for_each_possible_cpu(cpu) {
+ bpf_long_memcpy(per_cpu_ptr(storage->percpu_buf, cpu),
+ value + off, size);
+ off += size;
+ }
+ rcu_read_unlock();
+ return 0;
+}
+
static int cgroup_storage_get_next_key(struct bpf_map *_map, void *_key,
void *_next_key)
{
@@ -292,55 +357,98 @@ struct bpf_cgroup_storage *bpf_cgroup_storage_alloc(struct bpf_prog *prog,
{
struct bpf_cgroup_storage *storage;
struct bpf_map *map;
+ gfp_t flags;
+ size_t size;
u32 pages;
map = prog->aux->cgroup_storage[stype];
if (!map)
return NULL;
- pages = round_up(sizeof(struct bpf_cgroup_storage) +
- sizeof(struct bpf_storage_buffer) +
- map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
+ if (stype == BPF_CGROUP_STORAGE_SHARED) {
+ size = sizeof(struct bpf_storage_buffer) + map->value_size;
+ pages = round_up(sizeof(struct bpf_cgroup_storage) + size,
+ PAGE_SIZE) >> PAGE_SHIFT;
+ } else {
+ size = map->value_size;
+ pages = round_up(round_up(size, 8) * num_possible_cpus(),
+ PAGE_SIZE) >> PAGE_SHIFT;
+ }
+
if (bpf_map_charge_memlock(map, pages))
return ERR_PTR(-EPERM);
storage = kmalloc_node(sizeof(struct bpf_cgroup_storage),
__GFP_ZERO | GFP_USER, map->numa_node);
- if (!storage) {
- bpf_map_uncharge_memlock(map, pages);
- return ERR_PTR(-ENOMEM);
- }
+ if (!storage)
+ goto enomem;
- storage->buf = kmalloc_node(sizeof(struct bpf_storage_buffer) +
- map->value_size, __GFP_ZERO | GFP_USER,
- map->numa_node);
- if (!storage->buf) {
- bpf_map_uncharge_memlock(map, pages);
- kfree(storage);
- return ERR_PTR(-ENOMEM);
+ flags = __GFP_ZERO | GFP_USER;
+
+ if (stype == BPF_CGROUP_STORAGE_SHARED) {
+ storage->buf = kmalloc_node(size, flags, map->numa_node);
+ if (!storage->buf)
+ goto enomem;
+ } else {
+ storage->percpu_buf = __alloc_percpu_gfp(size, 8, flags);
+ if (!storage->percpu_buf)
+ goto enomem;
}
storage->map = (struct bpf_cgroup_storage_map *)map;
return storage;
+
+enomem:
+ bpf_map_uncharge_memlock(map, pages);
+ kfree(storage);
+ return ERR_PTR(-ENOMEM);
+}
+
+static void free_cgroup_storage_rcu(struct rcu_head *rcu)
+{
+ struct bpf_cgroup_storage *storage =
+ container_of(rcu, struct bpf_cgroup_storage, rcu);
+
+ kfree(storage->buf);
+ kfree(storage);
+}
+
+static void free_percpu_cgroup_storage_rcu(struct rcu_head *rcu)
+{
+ struct bpf_cgroup_storage *storage =
+ container_of(rcu, struct bpf_cgroup_storage, rcu);
+
+ free_percpu(storage->percpu_buf);
+ kfree(storage);
}
void bpf_cgroup_storage_free(struct bpf_cgroup_storage *storage)
{
- u32 pages;
+ enum bpf_cgroup_storage_type stype;
struct bpf_map *map;
+ u32 pages;
if (!storage)
return;
map = &storage->map->map;
- pages = round_up(sizeof(struct bpf_cgroup_storage) +
- sizeof(struct bpf_storage_buffer) +
- map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
+ stype = cgroup_storage_type(map);
+ if (stype == BPF_CGROUP_STORAGE_SHARED)
+ pages = round_up(sizeof(struct bpf_cgroup_storage) +
+ sizeof(struct bpf_storage_buffer) +
+ map->value_size, PAGE_SIZE) >> PAGE_SHIFT;
+ else
+ pages = round_up(round_up(map->value_size, 8) *
+ num_possible_cpus(),
+ PAGE_SIZE) >> PAGE_SHIFT;
+
bpf_map_uncharge_memlock(map, pages);
- kfree_rcu(storage->buf, rcu);
- kfree_rcu(storage, rcu);
+ if (stype == BPF_CGROUP_STORAGE_SHARED)
+ call_rcu(&storage->rcu, free_cgroup_storage_rcu);
+ else
+ call_rcu(&storage->rcu, free_percpu_cgroup_storage_rcu);
}
void bpf_cgroup_storage_link(struct bpf_cgroup_storage *storage,
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 8c91d2b41b1e..5742df21598c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -686,7 +686,8 @@ static int map_lookup_elem(union bpf_attr *attr)
if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
+ map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
value_size = round_up(map->value_size, 8) * num_possible_cpus();
else if (IS_FD_MAP(map))
value_size = sizeof(u32);
@@ -705,6 +706,8 @@ static int map_lookup_elem(union bpf_attr *attr)
err = bpf_percpu_hash_copy(map, key, value);
} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
err = bpf_percpu_array_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
+ err = bpf_percpu_cgroup_storage_copy(map, key, value);
} else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
err = bpf_stackmap_copy(map, key, value);
} else if (IS_FD_ARRAY(map)) {
@@ -774,7 +777,8 @@ static int map_update_elem(union bpf_attr *attr)
if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY)
+ map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
value_size = round_up(map->value_size, 8) * num_possible_cpus();
else
value_size = map->value_size;
@@ -809,6 +813,9 @@ static int map_update_elem(union bpf_attr *attr)
err = bpf_percpu_hash_update(map, key, value, attr->flags);
} else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
err = bpf_percpu_array_update(map, key, value, attr->flags);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
+ err = bpf_percpu_cgroup_storage_update(map, key, value,
+ attr->flags);
} else if (IS_FD_ARRAY(map)) {
rcu_read_lock();
err = bpf_fd_array_map_update_elem(map, f.file, key, value,
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e75f36de91d6..d94073deb68a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2074,6 +2074,7 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
goto error;
break;
case BPF_MAP_TYPE_CGROUP_STORAGE:
+ case BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE:
if (func_id != BPF_FUNC_get_local_storage)
goto error;
break;
@@ -2164,7 +2165,8 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
goto error;
break;
case BPF_FUNC_get_local_storage:
- if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE)
+ if (map->map_type != BPF_MAP_TYPE_CGROUP_STORAGE &&
+ map->map_type != BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
goto error;
break;
case BPF_FUNC_sk_select_reuseport:
@@ -5049,6 +5051,12 @@ static int check_map_prog_compatibility(struct bpf_verifier_env *env,
return 0;
}
+static bool bpf_map_is_cgroup_storage(struct bpf_map *map)
+{
+ return (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE);
+}
+
/* look for pseudo eBPF instructions that access map FDs and
* replace them with actual map pointers
*/
@@ -5139,10 +5147,9 @@ static int replace_map_fd_with_map_ptr(struct bpf_verifier_env *env)
}
env->used_maps[env->used_map_cnt++] = map;
- if (map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE &&
+ if (bpf_map_is_cgroup_storage(map) &&
bpf_cgroup_storage_assign(env->prog, map)) {
- verbose(env,
- "only one cgroup storage is allowed\n");
+ verbose(env, "only one cgroup storage of each type is allowed\n");
fdput(f);
return -EBUSY;
}
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 4/9] bpf: don't allow create maps of per-cpu cgroup local storages
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
Explicitly forbid creating map of per-cpu cgroup local storages.
This behavior matches the behavior of shared cgroup storages.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
kernel/bpf/map_in_map.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/map_in_map.c b/kernel/bpf/map_in_map.c
index 3bfbf4464416..99d243e1ad6e 100644
--- a/kernel/bpf/map_in_map.c
+++ b/kernel/bpf/map_in_map.c
@@ -24,7 +24,8 @@ struct bpf_map *bpf_map_meta_alloc(int inner_map_ufd)
* in the verifier is not enough.
*/
if (inner_map->map_type == BPF_MAP_TYPE_PROG_ARRAY ||
- inner_map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE) {
+ inner_map->map_type == BPF_MAP_TYPE_CGROUP_STORAGE ||
+ inner_map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
fdput(f);
return ERR_PTR(-ENOTSUPP);
}
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 5/9] bpf: sync include/uapi/linux/bpf.h to tools/include/uapi/linux/bpf.h
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
The sync is required due to the appearance of a new map type:
BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE, which implements per-cpu
cgroup local storage.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
tools/include/uapi/linux/bpf.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index aa5ccd2385ed..e2070d819e04 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -127,6 +127,7 @@ enum bpf_map_type {
BPF_MAP_TYPE_SOCKHASH,
BPF_MAP_TYPE_CGROUP_STORAGE,
BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
+ BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
};
enum bpf_prog_type {
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 6/9] bpftool: add support for PERCPU_CGROUP_STORAGE maps
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Jakub Kicinski, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
This commit adds support for BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE
map type.
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
tools/bpf/bpftool/map.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index af8ad32fa6e9..cb8177593a9a 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -71,13 +71,15 @@ static const char * const map_type_name[] = {
[BPF_MAP_TYPE_XSKMAP] = "xskmap",
[BPF_MAP_TYPE_SOCKHASH] = "sockhash",
[BPF_MAP_TYPE_CGROUP_STORAGE] = "cgroup_storage",
+ [BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE] = "percpu_cgroup_storage",
};
static bool map_is_per_cpu(__u32 type)
{
return type == BPF_MAP_TYPE_PERCPU_HASH ||
type == BPF_MAP_TYPE_PERCPU_ARRAY ||
- type == BPF_MAP_TYPE_LRU_PERCPU_HASH;
+ type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
+ type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE;
}
static bool map_is_map_of_maps(__u32 type)
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 8/9] selftests/bpf: extend the storage test to test per-cpu cgroup storage
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
This test extends the cgroup storage test to use per-cpu flavor
of the cgroup storage as well.
The test initializes a per-cpu cgroup storage to some non-zero initial
value (1000), and then simple bumps a per-cpu counter each time
the shared counter is atomically incremented. Then it reads all
per-cpu areas from the userspace side, and checks that the sum
of values adds to the expected sum.
Expected output:
$ ./test_cgroup_storage
test_cgroup_storage:PASS
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
.../selftests/bpf/test_cgroup_storage.c | 59 ++++++++++++++++++-
1 file changed, 56 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_cgroup_storage.c b/tools/testing/selftests/bpf/test_cgroup_storage.c
index 4e196e3bfecf..3dfc3773c790 100644
--- a/tools/testing/selftests/bpf/test_cgroup_storage.c
+++ b/tools/testing/selftests/bpf/test_cgroup_storage.c
@@ -4,6 +4,7 @@
#include <linux/filter.h>
#include <stdio.h>
#include <stdlib.h>
+#include <sys/sysinfo.h>
#include "bpf_rlimit.h"
#include "cgroup_helpers.h"
@@ -15,6 +16,14 @@ char bpf_log_buf[BPF_LOG_BUF_SIZE];
int main(int argc, char **argv)
{
struct bpf_insn prog[] = {
+ BPF_LD_MAP_FD(BPF_REG_1, 0), /* percpu map fd */
+ BPF_MOV64_IMM(BPF_REG_2, 0), /* flags, not used */
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_get_local_storage),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 0x1),
+ BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_3, 0),
+
BPF_LD_MAP_FD(BPF_REG_1, 0), /* map fd */
BPF_MOV64_IMM(BPF_REG_2, 0), /* flags, not used */
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
@@ -28,9 +37,18 @@ int main(int argc, char **argv)
};
size_t insns_cnt = sizeof(prog) / sizeof(struct bpf_insn);
int error = EXIT_FAILURE;
- int map_fd, prog_fd, cgroup_fd;
+ int map_fd, percpu_map_fd, prog_fd, cgroup_fd;
struct bpf_cgroup_storage_key key;
unsigned long long value;
+ unsigned long long *percpu_value;
+ int cpu, nproc;
+
+ nproc = get_nprocs_conf();
+ percpu_value = malloc(sizeof(*percpu_value) * nproc);
+ if (!percpu_value) {
+ printf("Not enough memory for per-cpu area (%d cpus)\n", nproc);
+ goto err;
+ }
map_fd = bpf_create_map(BPF_MAP_TYPE_CGROUP_STORAGE, sizeof(key),
sizeof(value), 0, 0);
@@ -39,7 +57,15 @@ int main(int argc, char **argv)
goto out;
}
- prog[0].imm = map_fd;
+ percpu_map_fd = bpf_create_map(BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
+ sizeof(key), sizeof(value), 0, 0);
+ if (percpu_map_fd < 0) {
+ printf("Failed to create map: %s\n", strerror(errno));
+ goto out;
+ }
+
+ prog[0].imm = percpu_map_fd;
+ prog[7].imm = map_fd;
prog_fd = bpf_load_program(BPF_PROG_TYPE_CGROUP_SKB,
prog, insns_cnt, "GPL", 0,
bpf_log_buf, BPF_LOG_BUF_SIZE);
@@ -77,7 +103,15 @@ int main(int argc, char **argv)
}
if (bpf_map_lookup_elem(map_fd, &key, &value)) {
- printf("Failed to lookup cgroup storage\n");
+ printf("Failed to lookup cgroup storage 0\n");
+ goto err;
+ }
+
+ for (cpu = 0; cpu < nproc; cpu++)
+ percpu_value[cpu] = 1000;
+
+ if (bpf_map_update_elem(percpu_map_fd, &key, percpu_value, 0)) {
+ printf("Failed to update the data in the cgroup storage\n");
goto err;
}
@@ -120,6 +154,25 @@ int main(int argc, char **argv)
goto err;
}
+ /* Check the final value of the counter in the percpu local storage */
+
+ for (cpu = 0; cpu < nproc; cpu++)
+ percpu_value[cpu] = 0;
+
+ if (bpf_map_lookup_elem(percpu_map_fd, &key, percpu_value)) {
+ printf("Failed to lookup the per-cpu cgroup storage\n");
+ goto err;
+ }
+
+ value = 0;
+ for (cpu = 0; cpu < nproc; cpu++)
+ value += percpu_value[cpu];
+
+ if (value != nproc * 1000 + 6) {
+ printf("Unexpected data in the per-cpu cgroup storage\n");
+ goto err;
+ }
+
error = 0;
printf("test_cgroup_storage:PASS\n");
--
2.17.1
^ permalink raw reply related
* [PATCH bpf-next 9/9] samples/bpf: extend test_cgrp2_attach2 test to use per-cpu cgroup storage
From: Roman Gushchin @ 2018-09-21 17:14 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Roman Gushchin,
Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <20180921171353.11050-1-guro@fb.com>
This commit extends the test_cgrp2_attach2 test to cover per-cpu
cgroup storage. Bpf program will use shared and per-cpu cgroup
storages simultaneously, so a better coverage of corresponding
core code will be achieved.
Expected output:
$ ./test_cgrp2_attach2
Attached DROP prog. This ping in cgroup /foo should fail...
ping: sendmsg: Operation not permitted
Attached DROP prog. This ping in cgroup /foo/bar should fail...
ping: sendmsg: Operation not permitted
Attached PASS prog. This ping in cgroup /foo/bar should pass...
Detached PASS from /foo/bar while DROP is attached to /foo.
This ping in cgroup /foo/bar should fail...
ping: sendmsg: Operation not permitted
Attached PASS from /foo/bar and detached DROP from /foo.
This ping in cgroup /foo/bar should pass...
### override:PASS
### multi:PASS
Signed-off-by: Roman Gushchin <guro@fb.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
---
samples/bpf/test_cgrp2_attach2.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/test_cgrp2_attach2.c b/samples/bpf/test_cgrp2_attach2.c
index 180f9d813bca..d7b68ef5ba79 100644
--- a/samples/bpf/test_cgrp2_attach2.c
+++ b/samples/bpf/test_cgrp2_attach2.c
@@ -209,7 +209,7 @@ static int map_fd = -1;
static int prog_load_cnt(int verdict, int val)
{
- int cgroup_storage_fd;
+ int cgroup_storage_fd, percpu_cgroup_storage_fd;
if (map_fd < 0)
map_fd = bpf_create_map(BPF_MAP_TYPE_ARRAY, 4, 8, 1, 0);
@@ -225,6 +225,14 @@ static int prog_load_cnt(int verdict, int val)
return -1;
}
+ percpu_cgroup_storage_fd = bpf_create_map(
+ BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
+ sizeof(struct bpf_cgroup_storage_key), 8, 0, 0);
+ if (percpu_cgroup_storage_fd < 0) {
+ printf("failed to create map '%s'\n", strerror(errno));
+ return -1;
+ }
+
struct bpf_insn prog[] = {
BPF_MOV32_IMM(BPF_REG_0, 0),
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
@@ -235,11 +243,20 @@ static int prog_load_cnt(int verdict, int val)
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
BPF_MOV64_IMM(BPF_REG_1, val), /* r1 = 1 */
BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
+
BPF_LD_MAP_FD(BPF_REG_1, cgroup_storage_fd),
BPF_MOV64_IMM(BPF_REG_2, 0),
BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
BPF_MOV64_IMM(BPF_REG_1, val),
BPF_RAW_INSN(BPF_STX | BPF_XADD | BPF_W, BPF_REG_0, BPF_REG_1, 0, 0),
+
+ BPF_LD_MAP_FD(BPF_REG_1, percpu_cgroup_storage_fd),
+ BPF_MOV64_IMM(BPF_REG_2, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_get_local_storage),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_0, 0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_3, 0x1),
+ BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_3, 0),
+
BPF_MOV64_IMM(BPF_REG_0, verdict), /* r0 = verdict */
BPF_EXIT_INSN(),
};
--
2.17.1
^ permalink raw reply related
* WARNING in clear_standby
From: syzbot @ 2018-09-21 17:24 UTC (permalink / raw)
To: ceph-devel, davem, idryomov, linux-kernel, netdev, sage,
syzkaller-bugs, zyan
Hello,
syzbot found the following crash on:
HEAD commit: 46c163a036b4 Add linux-next specific files for 20180921
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17e32b9e400000
kernel config: https://syzkaller.appspot.com/x/.config?x=20ea07a946ad19d7
dashboard link: https://syzkaller.appspot.com/bug?extid=acdeb633f6211ccdf886
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14ffa12a400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1668561a400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+acdeb633f6211ccdf886@syzkaller.appspotmail.com
libceph: mon0 [::1]:6789 socket error on write
libceph: mon0 [::1]:6789 socket error on write
libceph: mon0 [::1]:6789 socket error on write
libceph: mon0 [::1]:6789 socket error on write
libceph: mon0 [::1]:6789 socket error on write
WARNING: CPU: 1 PID: 5595 at net/ceph/messenger.c:3101
clear_standby+0x174/0x1b0 net/ceph/messenger.c:3101
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 5595 Comm: syz-executor575 Not tainted
4.19.0-rc4-next-20180921+ #77
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1d3/0x2c4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x254/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:969
RIP: 0010:clear_standby+0x174/0x1b0 net/ceph/messenger.c:3101
Code: 48 89 da 48 c7 c6 00 99 e3 88 48 c7 c7 e0 d3 4f 8a e8 a0 ee 02 fc e9
fe fe ff ff e8 06 e4 21 fa e9 47 ff ff ff e8 7c 6a de f9 <0f> 0b eb 85 4c
89 ef e8 10 e4 21 fa e9 ae fe ff ff 4c 89 ef e8 03
RSP: 0018:ffff8801bfb67710 EFLAGS: 00010293
RAX: ffff8801d928e5c0 RBX: ffff8801cbca2028 RCX: ffffffff879ee638
RDX: 0000000000000000 RSI: ffffffff879ee6b4 RDI: 0000000000000007
RBP: ffff8801bfb67728 R08: ffff8801d928e5c0 R09: ffffed0039794435
R10: ffff8801bfb67728 R11: ffff8801cbca21af R12: 0000000000000001
R13: ffff8801cbca2160 R14: ffff8801c878fb90 R15: ffff8801bfb67808
ceph_con_send+0x471/0x930 net/ceph/messenger.c:3136
__send_prepared_auth_request+0x155/0x1b0 net/ceph/mon_client.c:111
__open_session+0x3df/0x640 net/ceph/mon_client.c:199
ceph_monc_open_session+0x47/0x60 net/ceph/mon_client.c:448
__ceph_open_session+0xe2/0x830 net/ceph/ceph_common.c:705
ceph_real_mount fs/ceph/super.c:900 [inline]
ceph_mount+0x1410/0x1ea0 fs/ceph/super.c:1085
legacy_get_tree+0x131/0x460 fs/fs_context.c:718
vfs_get_tree+0x1cb/0x5c0 fs/super.c:1795
do_new_mount fs/namespace.c:2648 [inline]
do_mount+0x70c/0x1d90 fs/namespace.c:2974
ksys_mount+0x12d/0x140 fs/namespace.c:3190
__do_sys_mount fs/namespace.c:3204 [inline]
__se_sys_mount fs/namespace.c:3201 [inline]
__x64_sys_mount+0xbe/0x150 fs/namespace.c:3201
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x440d39
Code: e8 cc ab 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 bb 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdf06babe8 EFLAGS: 00000286 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0030656c69662f2e RCX: 0000000000440d39
RDX: 00000000200001c0 RSI: 0000000020000180 RDI: 0000000020000240
RBP: 0000000000000000 R08: 0000000000000000 R09: 00000009004002c8
R10: 0000000000000000 R11: 0000000000000286 R12: 000000000000956a
R13: 0000000000401d10 R14: 0000000000000000 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* KMSAN: uninit-value in tipc_nl_compat_dumpit
From: syzbot @ 2018-09-21 17:24 UTC (permalink / raw)
To: davem, jon.maloy, linux-kernel, netdev, syzkaller-bugs,
tipc-discussion, ying.xue
Hello,
syzbot found the following crash on:
HEAD commit: 74ee2200b89f kmsan: bump .config.example to v4.17-rc3
git tree: https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=1164f197800000
kernel config: https://syzkaller.appspot.com/x/.config?x=4ca1e57bafa8ab1f
dashboard link: https://syzkaller.appspot.com/bug?extid=6bdb590321a7ae40c1a6
compiler: clang version 7.0.0 (trunk 329391)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15341157800000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17410147800000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+6bdb590321a7ae40c1a6@syzkaller.appspotmail.com
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
==================================================================
BUG: KMSAN: uninit-value in tipc_nl_compat_dumpit+0x490/0x870
net/tipc/netlink_compat.c:245
CPU: 0 PID: 4507 Comm: syz-executor018 Not tainted 4.17.0-rc3+ #88
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x185/0x1d0 lib/dump_stack.c:113
kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
__msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
tipc_nl_compat_dumpit+0x490/0x870 net/tipc/netlink_compat.c:245
tipc_nl_compat_handle net/tipc/netlink_compat.c:1108 [inline]
tipc_nl_compat_recv+0x1394/0x2700 net/tipc/netlink_compat.c:1204
genl_family_rcv_msg net/netlink/genetlink.c:599 [inline]
genl_rcv_msg+0x1686/0x1810 net/netlink/genetlink.c:624
netlink_rcv_skb+0x378/0x600 net/netlink/af_netlink.c:2448
genl_rcv+0x63/0x80 net/netlink/genetlink.c:635
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x166b/0x1740 net/netlink/af_netlink.c:1336
netlink_sendmsg+0x1072/0x1370 net/netlink/af_netlink.c:1901
sock_sendmsg_nosec net/socket.c:629 [inline]
sock_sendmsg net/socket.c:639 [inline]
___sys_sendmsg+0xec0/0x1310 net/socket.c:2117
__sys_sendmsg net/socket.c:2155 [inline]
__do_sys_sendmsg net/socket.c:2164 [inline]
__se_sys_sendmsg net/socket.c:2162 [inline]
__x64_sys_sendmsg+0x331/0x460 net/socket.c:2162
do_syscall_64+0x154/0x220 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x445599
RSP: 002b:00007f4c1ecc6db8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00000000006dac24 RCX: 0000000000445599
RDX: 0000000000000000 RSI: 0000000020023000 RDI: 0000000000000003
RBP: 00000000006dac20 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007ffe5ef31bcf R14: 00007f4c1ecc79c0 R15: 0000000000000001
Uninit was created at:
kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
kmsan_slab_alloc+0x10/0x20 mm/kmsan/kmsan.c:321
slab_post_alloc_hook mm/slab.h:446 [inline]
slab_alloc_node mm/slub.c:2753 [inline]
__kmalloc_node_track_caller+0xb32/0x11b0 mm/slub.c:4395
__kmalloc_reserve net/core/skbuff.c:138 [inline]
__alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
alloc_skb include/linux/skbuff.h:988 [inline]
netlink_alloc_large_skb net/netlink/af_netlink.c:1182 [inline]
netlink_sendmsg+0x76e/0x1370 net/netlink/af_netlink.c:1876
sock_sendmsg_nosec net/socket.c:629 [inline]
sock_sendmsg net/socket.c:639 [inline]
___sys_sendmsg+0xec0/0x1310 net/socket.c:2117
__sys_sendmsg net/socket.c:2155 [inline]
__do_sys_sendmsg net/socket.c:2164 [inline]
__se_sys_sendmsg net/socket.c:2162 [inline]
__x64_sys_sendmsg+0x331/0x460 net/socket.c:2162
do_syscall_64+0x154/0x220 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* [PATCH] RDS: IB: Use DEFINE_PER_CPU_SHARED_ALIGNED for rds_ib_stats
From: Nathan Chancellor @ 2018-09-21 18:04 UTC (permalink / raw)
To: Santosh Shilimkar, David S. Miller
Cc: netdev, linux-rdma, linux-kernel, Nick Desaulniers,
Nathan Chancellor
Clang warns when two declarations' section attributes don't match.
net/rds/ib_stats.c:40:1: warning: section does not match previous
declaration [-Wsection]
DEFINE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
^
./include/linux/percpu-defs.h:142:2: note: expanded from macro
'DEFINE_PER_CPU_SHARED_ALIGNED'
DEFINE_PER_CPU_SECTION(type, name,
PER_CPU_SHARED_ALIGNED_SECTION) \
^
./include/linux/percpu-defs.h:93:9: note: expanded from macro
'DEFINE_PER_CPU_SECTION'
extern __PCPU_ATTRS(sec) __typeof__(type) name;
\
^
./include/linux/percpu-defs.h:49:26: note: expanded from macro
'__PCPU_ATTRS'
__percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))
\
^
net/rds/ib.h:446:1: note: previous attribute is here
DECLARE_PER_CPU(struct rds_ib_statistics, rds_ib_stats);
^
./include/linux/percpu-defs.h:111:2: note: expanded from macro
'DECLARE_PER_CPU'
DECLARE_PER_CPU_SECTION(type, name, "")
^
./include/linux/percpu-defs.h:87:9: note: expanded from macro
'DECLARE_PER_CPU_SECTION'
extern __PCPU_ATTRS(sec) __typeof__(type) name
^
./include/linux/percpu-defs.h:49:26: note: expanded from macro
'__PCPU_ATTRS'
__percpu __attribute__((section(PER_CPU_BASE_SECTION sec)))
\
^
1 warning generated.
The initial definition was added in commit ec16227e1414 ("RDS/IB:
Infiniband transport") and the cache aligned definition was added in
commit e6babe4cc4ce ("RDS/IB: Stats and sysctls") right after. The
definition probably should have been updated in net/rds/ib.h, which is
what this patch does.
Link: https://github.com/ClangBuiltLinux/linux/issues/114
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
net/rds/ib.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/rds/ib.h b/net/rds/ib.h
index 73427ff439f9..fd483760c910 100644
--- a/net/rds/ib.h
+++ b/net/rds/ib.h
@@ -443,7 +443,7 @@ int rds_ib_send_grab_credits(struct rds_ib_connection *ic, u32 wanted,
int rds_ib_xmit_atomic(struct rds_connection *conn, struct rm_atomic_op *op);
/* ib_stats.c */
-DECLARE_PER_CPU(struct rds_ib_statistics, rds_ib_stats);
+DEFINE_PER_CPU_SHARED_ALIGNED(struct rds_ib_statistics, rds_ib_stats);
#define rds_ib_stats_inc(member) rds_stats_inc_which(rds_ib_stats, member)
#define rds_ib_stats_add(member, count) \
rds_stats_add_which(rds_ib_stats, member, count)
--
2.19.0
^ permalink raw reply related
* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Arnaldo Carvalho de Melo @ 2018-09-21 12:25 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Peter Zijlstra, Alexei Starovoitov, David S . Miller, daniel,
netdev, kernel-team
In-Reply-To: <20180921031444.fyem2ah6emimv24m@ast-mbp>
Em Thu, Sep 20, 2018 at 08:14:46PM -0700, Alexei Starovoitov escreveu:
> On Thu, Sep 20, 2018 at 03:56:51PM +0200, Peter Zijlstra wrote:
> > On Thu, Sep 20, 2018 at 10:25:45AM -0300, Arnaldo Carvalho de Melo wrote:
> > > PeterZ provided a patch introducing PERF_RECORD_MUNMAP, went nowhere due
> > > to having to cope with munmapping parts of existing mmaps, etc.
> > >
> > > I'm still more in favour of introduce PERF_RECORD_MUNMAP, even if for
> > > now it would be used just in this clean case for undoing a
> > > PERF_RECORD_MMAP for a BPF program.
> > >
> > > The ABI is already complicated, starting to use something called
> > > PERF_RECORD_MMAP for unmmaping by just using a NULL name... too clever,
> > > I think.
> >
> > Agreed, the PERF_RECORD_MUNMAP patch was fairly trivial, the difficult
> > part was getting the perf tool to dtrt for that use-case. But if we need
> > unmap events, doing the unmap record now is the right thing.
>
> Thanks for the pointers!
> The answer is a bit long. pls bear with me.
Ditto with me :-)
> I have considered adding MUNMAP to match existing MMAP, but went
> without it because I didn't want to introduce new bit in perf_event_attr
> and emit these new events in a misbalanced conditional way for prog load/unload.
> Like old perf is asking kernel for mmap events via mmap bit, so prog load events
By prog load events you mean that old perf, having perf_event_attr.mmap = 1 ||
perf_event_attr.mmap2 = 1 will cause the new kernel to emit
PERF_RECORD_MMAP records for the range of addresses that a BPF program
is being loaded on, right?
> will be in perf.data, but old perf report won't recognize them anyway.
Why not? It should lookup the symbol and find it in the rb_tree of maps,
with a DSO name equal to what was in the PERF_RECORD_MMAP emitted by the
BPF core, no? It'll be an unresolved symbol, but a resolved map.
> Whereas new perf would certainly want to catch bpf events and will set
> both mmap and mumap bits.
new perf with your code will find a symbol, not a map, because your code
catches a special case PERF_RECORD_MMAP and instead of creating a
'struct map' will create a 'struct symbol' and insert it in the kallsyms
'struct map', right?
> Then if I add MUNMAP event without new bit and emit MMAP/MUNMAP
> conditionally based on single mmap bit they will confuse old perf
> and it will print warning about 'unknown events'.
That is unfortunate and I'll turn that part into a pr_debug()
> Both situations are ugly, hence I went with reuse of MMAP event
> for both load/unload.
So, its doubly odd, i.e. MMAP used for mmap() and for munmap() and the
effects in the tooling is not to create or remove a 'struct map', but to
alter an existing symbol table for the kallsyms map.
> In such case old perf silently ignores them. Which is what I wanted.
In theory the old perf should catch the PERF_RECORD_MMAP with a string
in the filename part and insert a new map into the kernel mmap rb_tree,
and then samples would be resolved to this map, but since there is no
backing DSO with a symtab, it would stop at that, just stating that the
map is called NAME-OF-BPF-PROGRAM. This is all from memory, possibly
there is something in there that makes it ignore this PERF_RECORD_MMAP
emitted by the BPF kernel code when loading a new program.
> When we upgrade the kernel we cannot synchronize the kernel upgrade
> (or downgrade) with user space perf package upgrade.
sure
> Hence not confusing old perf is important.
Thanks for trying to achieve that, and its a pity that that "unknown
record" message is a pr_warning or pr_info and not a pr_debug().
> With new kernel new bpf mmap events get into perf.data and
> new perf picks them up.
>
> Few more considerations:
>
> I consider synthetic perf events to be non-ABI. Meaning they're
> emitted by perf user space into perf.data and there is a convention
> on names, but it's not a kernel abi. Like RECORD_MMAP with
> event.filename == "[module_name]" is an indication for perf report
> to parse elf/build-id of dso==module_name.
> There is no such support in the kernel. Kernel doesn't emit
> such events for module load/unload. If in the future
> we decide to extend kernel with such events they don't have
> to match what user space perf does today.
Right, that is another unfortunate state of affairs, kernel module
load/unload should already be supported, reported by the kernel via a
proper PERF_RECORD_MODULE_LOAD/UNLOAD
> Why this is important? To get to next step.
> As Arnaldo pointed out this patch set is missing support for
> JITed prog annotations and displaying asm code. Absolutely correct.
> This set only helps perf to reveal the names of bpf progs that _were_
> running at the time of perf record, but there is no way yet for
> perf report to show asm code of the prog that was running.
> In that sense bpf is drastically different from java, other jits
> and normal profiling.
> bpf JIT happens in the kernel and only kernel knows the mapping
> between original source and JITed code.
> In addition there are bpf2bpf functions. In the future there will
> be bpf libraries, more type info, line number support, etc.
> I strongly believe perf RECORD_* events should NOT care about
> the development that happens on the bpf side.
> The only thing kernel will be telling user space is that bpf prog
> with prog_id=X was loaded.
> Then based on prog_id the 'perf record' phase can query the kernel
> for bpf related information. There is already a way to fetch
> JITed image based on prog_id.
> Then perf will emit synthetic RECORD_FOOBAR into perf.data
> that will contain bpf related info (like complete JITed image)
> and perf report can process it later and annotate things in UI.
There is another longstanding TODO list entry: PERF_RECORD_MMAP records
should include a build-id, to avoid either userspace getting confused
when there is an update of some mmap DSO, for long running sessions, for
instance, or to have to scan the just recorded perf.data file for DSOs
with samples to then read it from the file system (more races).
Have you ever considered having a build-id for bpf objects that could be
used here?
> It may seem that there is a race here.
> Like when 'perf record' see 'bpf prog was loaded with prog_id=X' event
> it will ask the kernel about prog_id=X, but that prog could be
> unloaded already.
> In such case prog_id will not exist and perf record can ignore such event.
> So no race.
> The kernel doesn't need to pass all information about bpf prog to
> the user space via RECORD_*. Instead 'perf record' can emit
> synthetic events into perf.data.
> I was thinking to extend RECORD_MMAP with prog_id already
Right, if prog_id is a uniquely identifying cookie that is based on the
file contents, like files in a git repo, then this would serve for both
bpf progs and for everything else.
Will the JITed code from some BPF bytecode be different if the same
bytecode is JITed in several machines, all having the exact same
hardware?
> (instead of passing kallsyms's bpf prog name in event->mmap.filename)
> but bpf functions don't have their own prog_id. Multiple bpf funcs
> with different JITed blobs are considered to be a part of single prog_id.
> So as a step one I'm only extending RECORD_MMAP with addr and kallsym
> name of bpf function/prog.
> As a step two the plan is to add notification mechanism for prog load/unload
> that will include prog_id and design new synthetic RECORD_* events in
> perf user space that will contain JITed code, line info, BTF, etc.
So, will the kernel JIT a bytecode, load it somewhere and run it, then,
when unloading it, keep it somewhere (a filesystem with some limits,
etc) so that at some later time (with some timeouts) tooling can, using
its id/buildid cookie get the contents and symbol table to have a better
annotation experience?
Using /proc/kcore as right now we should be able to annotate the BPF JIT
as it is loaded, in 'perf top', for instance, with your suggested code,
because we would have a symbol resolved that spans the whole bpf
program, I'll try applying your patch and seeing how it goes.
> TLDR:
> step 1 (this patch set)
> Single bpf prog_load can call multiple
> bpf_prog_kallsyms_add() -> RECORD_MMAP with addr+kallsym only
> Similarly unload calls multiple
> bpf_prog_kallsyms_del() -> RECORD_MMAP with addr only
>
> step 2 (future work)
> single event for bpf prog_load with prog_id only.
> Either via perf ring buffer or ftrace or tracepoints or some
> other notification mechanism.
>
> It may seem that step 2 obsoletes step 1. It can, but I think
> it will complement it. There is a lot more code there and
> a lot more discussions to have.
> Step 1 is already big improvement.
>
> Thoughts?
See above, and, bear with me too :-)
- Arnaldo
^ permalink raw reply
* [PATCH net-next,v2,0/3] hv_netvsc: Support LRO/RSC in the vSwitch
From: Haiyang Zhang @ 2018-09-21 18:20 UTC (permalink / raw)
To: davem, netdev
Cc: haiyangz, kys, sthemmin, olaf, vkuznets, devel, linux-kernel
From: Haiyang Zhang <haiyangz@microsoft.com>
The patch adds support for LRO/RSC in the vSwitch feature. It reduces
the per packet processing overhead by coalescing multiple TCP segments
when possible. The feature is enabled by default on VMs running on
Windows Server 2019 and later.
The patch set also adds ethtool command handler and documents.
Haiyang Zhang (3):
hv_netvsc: Add support for LRO/RSC in the vSwitch
hv_netvsc: Add handler for LRO setting change
hv_netvsc: Update document for LRO/RSC support
Documentation/networking/netvsc.txt | 9 +++
drivers/net/hyperv/hyperv_net.h | 51 ++++++++++++---
drivers/net/hyperv/netvsc.c | 18 ++++--
drivers/net/hyperv/netvsc_drv.c | 58 ++++++++++++-----
drivers/net/hyperv/rndis_filter.c | 97 +++++++++++++++++++++++++----
5 files changed, 194 insertions(+), 39 deletions(-)
--
2.18.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox