* [PATCH iproute2 0/2] fix "ip link show dev ..." for NICs with many VFs
From: Michal Kubecek @ 2017-09-01 16:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, linux-kernel
Two of our customers recently encountered problems with processing of large
messages produced by kernel in response to "ip link show" for NICs with
many (120-128) virtual functions. While some of them have been already
addressed in recent versions of iproute2, some still persist.
Patch 1 adds check to handle the case when a message fits into the
buffer in rtnl_talk() but not into the buffer in iplink_get().
Patch 2 increases the buffer size in iplink_get() to suffice even for
NICs with 128 VFs.
Note: after applying patch 2, patch 1 seems useless as both buffers have
the same size so that the check cannot actually trigger. However, as we
cannot guarantee they will always stay the same, I believe the check
should still be added.
Michal Kubecek (2):
iplink: check for message truncation in iplink_get()
iplink: double the buffer size also in iplink_get()
ip/iplink.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
--
2.14.1
^ permalink raw reply
* [PATCH iproute2 1/2] iplink: check for message truncation in iplink_get()
From: Michal Kubecek @ 2017-09-01 16:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1504283683.git.mkubecek@suse.cz>
If message length exceeds maxlen argument of rtnl_talk(), it is truncated
to maxlen but unlike in the case of truncation to the length of local
buffer in rtnl_talk(), the caller doesn't get any indication of a problem.
In particular, iplink_get() passes the truncated message on and parsing it
results in various warnings and sometimes even a segfault (observed with
"ip link show dev ..." for a NIC with 125 VFs).
Handle message truncation in iplink_get() the same way as truncation in
rtnl_talk() would be handled: return an error.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
ip/iplink.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/ip/iplink.c b/ip/iplink.c
index 5aff2fde38da..790e3a138bb0 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1040,6 +1040,11 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
if (rtnl_talk(&rth, &req.n, &answer.n, sizeof(answer)) < 0)
return -2;
+ if (answer.n.nlmsg_len > sizeof(answer.buf)) {
+ fprintf(stderr, "Message truncated from %u to %lu\n",
+ answer.n.nlmsg_len, sizeof(answer.buf));
+ return -2;
+ }
if (brief)
print_linkinfo_brief(NULL, &answer.n, stdout, NULL);
--
2.14.1
^ permalink raw reply related
* [PATCH iproute2 2/2] iplink: double the buffer size also in iplink_get()
From: Michal Kubecek @ 2017-09-01 16:39 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1504283683.git.mkubecek@suse.cz>
Commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") increased
the buffer size for "ip link show" command to 32 KB to handle NICs with
large number of VFs. With "dev" filter, a different code path is taken and
iplink_get() still uses only 16 KB buffer.
The size of 32768 is not very future-proof as NICs supporting 120-128 VFs
are already in use so that single RTM_NEWLINK message in the dump can
exceed 30000 bytes. But it's what rtnl_talk() and rtnl_dump_filter_l() use
so let's be consistent. Once this proves insufficient, all three sizes
should be increased.
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
ip/iplink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ip/iplink.c b/ip/iplink.c
index 790e3a138bb0..72c347932068 100644
--- a/ip/iplink.c
+++ b/ip/iplink.c
@@ -1024,7 +1024,7 @@ int iplink_get(unsigned int flags, char *name, __u32 filt_mask)
};
struct {
struct nlmsghdr n;
- char buf[16384];
+ char buf[32768];
} answer;
if (name) {
--
2.14.1
^ permalink raw reply related
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Florian Fainelli @ 2017-09-01 16:46 UTC (permalink / raw)
To: Andrew Lunn, jiri, jhs; +Cc: netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <20170901132921.GV22289@lunn.ch>
On 09/01/2017 06:29 AM, Andrew Lunn wrote:
>> I suppose that you could somehow use TC to influence how the traffic
>> from host to CPU works, but without a "CPU" port representor the
>> question is how do we get that done? If we used "eth0" we need to
>> callback into the switch driver for programming..
>
> We need to compare how the different switches work with respect to
> QoS. Marvell switches do a lot of the classification on the ingress
> port where it determines what queue the frame should be placed in on
> the egress port. The egress port then schedules its queues.
>
> This does not map to TC too well.
>
>> Regarding the last patch in this series, what I would ideally to replace
>> it with is something along the lines of:
>>
>> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>
> Why do you need this? sw0p0 has 8 queues? So i assume you use TC on
> sw0p0 to place frames into these queues? The queue then gets passed
> transparently down through the conduit interface and then used by the
> tagger. I don't see why you need eth0 here? We try our best to avoid
> eth0 wherever possible, it causes confusion. So i would prefer not to
> have to use eth0 with TC commands.
Well, if you read through patch 8 maybe this is explained. The dynamic
queue selection is working fine through the use of the DSA network
device's queue being passed to the Broadcom tag. If there was just I
would agree with you, but here is the catch below.
We also have this unique (AFAICT) hardware feature called Advanced
Congestion Buffering (ACB) where the CPU Ethernet MAC can receive
congestion information from the switch queues directly, but out of band
from specifically added HW logic and signals. This is not using pause
frames.
This is useful for instance when your CPU is linking at 1Gbits/sec (or
more) internally with the switch, but you have connected external hosts
that are only 10/100 capable. When you push 1Gbits/sec of traffic
towards such hosts, you would get severe packet loss, unless you have
pause frames enabled. The problem with Pause frames within the SF2
switch design is that they are not per-flow though, so you can't resolve
from the pause frames which egress queue to backpressure. With ACB
though, you get end-to-end backpressure between say, Port 8 and Port 0.
In order for this to work though, you need the CPU MAC to be inspecting
or rather receiving congestion notification from the switch port and
queues directly.
This is why I need to define a mapping between switch (port P,queue Q)
and CPU MAC queues (Q'). In the first generation HW, we have up to 4
ports exposed, each with 8 queues, and we have 32 CPU queues, 1:1
mapping is possible. On 2nd generation hardware, same number of ports
and queues per port, but only 16 queues, so a 2:1 mapping is possible only.
If I do not establish the mapping, several problems can occur but the
most severe is that congestion notification logic gets its congestion
information from the wrong port and queue, so it can be permanently
backpressuring the CPU queue based on e.g: a disabled port. This results
in the transmit queue being disabled, and so you get the netdev watchdog
to kick in and scream.
As you can see from the last patch, I used a notifier to receive
information when DSA slave network devices are added, from which I
extract their port number and set up an internal mapping to the CPU
queues, but this is creating a layering violation since I have now the
CPU driver extracting DSA specific net_device information.
The idea behind exposing a command like the proposed "tc bind" is to let
users define the mapping as they see fit. 1:1 or 2:1 mapping is fine as
a default, but may not be satisfactory for some use cases.
Hope this helps understand the bigger picture ;)
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/2] flow_dissector: Add limits for encapsulation and EH
From: Tom Herbert @ 2017-09-01 16:49 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Tom Herbert, David S . Miller, Linux Kernel Network Developers,
alex.popov
In-Reply-To: <87tw0mjs11.fsf@stressinduktion.org>
On Fri, Sep 1, 2017 at 9:35 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Hello Tom,
>
> Tom Herbert <tom@quantonium.net> writes:
>
>> On Fri, Sep 1, 2017 at 6:32 AM, Hannes Frederic Sowa
>> <hannes@stressinduktion.org> wrote:
>>> Tom Herbert <tom@quantonium.net> writes:
>>>
>>>> In flow dissector there are no limits to the number of nested
>>>> encapsulations that might be dissected which makes for a nice DOS
>>>> attack. This patch limits for dissecting nested encapsulations
>>>> as well as for dissecting over extension headers.
>>>
>>> I was actually more referring to your patch, because the flow dissector
>>> right now is not stack recursive. Your changes would make it doing
>>> recursion on the stack.
>>
>> I don't believe those patches had any recursion.
>
> I was wrong with stack recursion, you handle that using the
> FLOW_DISSECT_RET_PROTO_AGAIN return value thus leaving the stack frame
> again, sorry.
>
> But otherwise the walk would be unlimited (based on the packet size) in
> your first patchset, correct? See this malicious example:
>
> | IP1 | UDP1 | VXLAN1 | Ethernet | IP2 | UDP2 | VXLAN2 | ...
>
Without the limits patch I subsequently proposed, yes. However, this
is true for all the other encapsulations anyway; there's is nothing
unique about UDP encapsulations in this regard (hence with the limit
patch should generally apply to all encapsulations).
> where IP1 == IP2, UDP1 == UDP2 and VXLAN1 != VXLAN2?
>
> Notice that because IP1 == IP2 and UDP1 == UDP2 it seems to me it would
> hit the same socket again. We would be prone to overwrite vxlan id 1
> with vxlan id 2 in the key thus the key would be malicious and traffic
> could be injected into other tenant networks, if the encapsulated
> packets within VXLAN1 could be generated by a malicious user?
>
This is why flow dissection is not an authoritative parsing of the
packet. It can be wrong or misleading because it doesn't have all the
context, doesn't necessarily parse the whole chain, and only returns
one set of information (for instance only one pair of IP addresses
when there may be more in a packet). It's just a best effort mechanism
that is great for computing a hash for instance. If someone is
steering a packet to a VM based on the output of flow dissector that
is a bug; the only correct way to do this is to go through the normal
receive protocol processing path.
> I was actually not concerned about the "recursion" but merely about
> updating the values to the innermost values.
>
See my previous comment about use STOP_AT_ENCAP.
Tom
>>>> Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>>>> Signed-off-by: Tom Herbert <tom@quantonium.net>
>>>> ---
>>>> net/core/flow_dissector.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---
>>>> 1 file changed, 45 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
>>>> index 5110180a3e96..1bca748de27d 100644
>>>> --- a/net/core/flow_dissector.c
>>>> +++ b/net/core/flow_dissector.c
>>>> @@ -396,6 +396,35 @@ __skb_flow_dissect_ipv6(const struct sk_buff *skb,
>>>> key_ip->ttl = iph->hop_limit;
>>>> }
>>>>
>>>> +/* Maximum number of nested encapsulations that can be processed in
>>>> + * __skb_flow_dissect
>>>> + */
>>>> +#define MAX_FLOW_DISSECT_ENCAPS 5
>>>
>>> I think you can exactly parse one encapsulation layer safely. This is
>>> because you can only keep state on one encapsulation layer protocol
>>> right now.
>>>
>>> For example this scenario:
>>>
>>> I would like to circumvent tc flower rules that filter base
>>> simply construct a packet looking like:
>>>
>>> Ethernet|Vlan|IP|GRE|Ethernet|Vlan|
>>>
>>> because we don't recurse in the flow keys either, the second vlan header
>>> would overwrite the information of the first one.
>>>
>> Flow dissector returns the inner most information it sees. If only
>> information from the outermost headers is needed then the caller
>> should set FLOW_DISSECTOR_F_STOP_AT_ENCAP in the flags.
>
> Right now, flower does not do so and seems to be prone to be fooled like
> that.
>
> Additionally, flow_dissector does not update all keys to its inner most
> values also, e.g. I don't see the inner packets updating
> FLOW_DISSECTOR_KEY_ETH_ADDRS (e.g. with ETH_P_TEB).
>
> To me it would make more sense to be able to specificy
> FLOW_DISSECTOR_F_AFTER_FIRST_ENCAP, so that we get the first
> encapsulation information with the key, do policy decision, pull headers
> and recurse here.
>
>>>> +
>>>> +static bool skb_flow_dissect_encap_allowed(int *num_encaps, unsigned int *flags)
>>>> +{
>>>> + ++*num_encaps;
>>>> +
>>>> + if (*num_encaps >= MAX_FLOW_DISSECT_ENCAPS) {
>>>> + if (*num_encaps == MAX_FLOW_DISSECT_ENCAPS) {
>>>> + /* Allow one more pass but ignore disregard
>>>> + * further encapsulations
>>>> + */
>>>> + *flags |= FLOW_DISSECTOR_F_STOP_AT_ENCAP;
>>>> + } else {
>>>> + /* Max encaps reached */
>>>> + return false;
>>>> + }
>>>> + }
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> +/* Maximum number of extension headers can be processed in __skb_flow_dissect
>>>> + * per IPv6 packet
>>>> + */
>>>> +#define MAX_FLOW_DISSECT_EH 5
>>>
>>> I would at least allow each extension header once, DEST_OPS twice, thus
>>> let's say 12? It is safe in this version because it does not consume
>>> stack space anyway and doesn't update internal state.
>>
>> This per each IPv6 header. This patch would allow up to five
>> encapsulations of IPv6/IPv6 so maximum number of EHs we would consider
>> is 6*5=30.
>
> I would still increase the limit, but also no hard feelings. The reason
> I came up with 12 is that anyway each header (besides DST_OPS) is only
> allowed to show up once per header (11 + 1).
>
> I am fine that this is a per-IPv6 header limit.
>
>>>
>>>> +
>>>> /**
>>>> * __skb_flow_dissect - extract the flow_keys struct and return it
>>>> * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified
>>>> @@ -426,6 +455,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>>> struct flow_dissector_key_tags *key_tags;
>>>> struct flow_dissector_key_vlan *key_vlan;
>>>> enum flow_dissect_ret fdret;
>>>> + int num_eh, num_encaps = 0;
>>>> bool skip_vlan = false;
>>>> u8 ip_proto = 0;
>>>> bool ret;
>>>> @@ -714,7 +744,9 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>>>> case FLOW_DISSECT_RET_OUT_GOOD:
>>>> goto out_good;
>>>> case FLOW_DISSECT_RET_PROTO_AGAIN:
>>>> - goto proto_again;
>>>> + if (skb_flow_dissect_encap_allowed(&num_encaps, &flags))
>>>> + goto proto_again;
>>>
>>> I think you should get the check to the proto_again label. In case you
>>> loop to often you can `goto out_good'.
>>>
>> That would add an extra conditional in the common case of no
>> encapsulation. Having it here means we only care about the
>> encapsulation limit when there is encapsulation.
>
> In my opinion it would make the limits easier to grasp, but no hard
> feelings about that.
>
> Thanks,
> Hannes
^ permalink raw reply
* [iproute PATCH 0/6] strlcpy() and strlcat() for iproute2
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
The following series adds my own implementations of strlcpy() and
strlcat() in patch 1 and changes the code to make use of them in the
following patches but the last two: Patch 5 just eliminates a line of
useless code I found while searching for potential users of the
introduced functions, patch 6 sanitizes a call to strncpy() in
misc/lnstat_util.c without using strlcpy() since lnstat is not being
linked against libutil.
I implemented both functions solely based on information in libbsd's man
pages, so they are safe to be released under the GPL.
Phil Sutter (6):
utils: Implement strlcpy() and strlcat()
Convert the obvious cases to strlcpy()
Convert harmful calls to strncpy() to strlcpy()
ipxfrm: Replace STRBUF_CAT macro with strlcat()
tc_util: No need to terminate an snprintf'ed buffer
lnstat_util: Make sure buffer is NUL-terminated
genl/ctrl.c | 2 +-
include/utils.h | 3 +++
ip/ipnetns.c | 3 +--
ip/iproute_lwtunnel.c | 3 +--
ip/ipvrf.c | 5 ++---
ip/ipxfrm.c | 21 +++++----------------
ip/xfrm_state.c | 2 +-
lib/bpf.c | 3 +--
lib/fs.c | 3 +--
lib/inet_proto.c | 3 +--
lib/utils.c | 19 +++++++++++++++++++
misc/lnstat_util.c | 3 ++-
misc/ss.c | 3 +--
tc/em_ipset.c | 3 +--
tc/tc_util.c | 1 -
15 files changed, 40 insertions(+), 37 deletions(-)
--
2.13.1
^ permalink raw reply
* [iproute PATCH 1/6] utils: Implement strlcpy() and strlcat()
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
By making use of strncpy(), both implementations are really simple so
there is no need to add libbsd as additional dependency.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
include/utils.h | 3 +++
lib/utils.c | 19 +++++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/include/utils.h b/include/utils.h
index f665d9001806f..9c2f9fc257fba 100644
--- a/include/utils.h
+++ b/include/utils.h
@@ -252,4 +252,7 @@ int make_path(const char *path, mode_t mode);
char *find_cgroup2_mount(void);
int get_command_name(const char *pid, char *comm, size_t len);
+size_t strlcpy(char *dst, const char *src, size_t size);
+size_t strlcat(char *dst, const char *src, size_t size);
+
#endif /* __UTILS_H__ */
diff --git a/lib/utils.c b/lib/utils.c
index 002063075fd61..c95780e725252 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -1238,3 +1238,22 @@ int get_real_family(int rtm_type, int rtm_family)
return rtm_family;
}
+
+size_t strlcpy(char *dst, const char *src, size_t size)
+{
+ if (size) {
+ strncpy(dst, src, size - 1);
+ dst[size - 1] = '\0';
+ }
+ return strlen(src);
+}
+
+size_t strlcat(char *dst, const char *src, size_t size)
+{
+ size_t dlen = strlen(dst);
+
+ if (dlen > size)
+ return dlen + strlen(src);
+
+ return dlen + strlcpy(dst + dlen, src, size - dlen);
+}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 4/6] ipxfrm: Replace STRBUF_CAT macro with strlcat()
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ipxfrm.c | 21 +++++----------------
1 file changed, 5 insertions(+), 16 deletions(-)
diff --git a/ip/ipxfrm.c b/ip/ipxfrm.c
index d5eb22e25476a..12c2f721571b6 100644
--- a/ip/ipxfrm.c
+++ b/ip/ipxfrm.c
@@ -40,17 +40,6 @@
#include "ip_common.h"
#define STRBUF_SIZE (128)
-#define STRBUF_CAT(buf, str) \
- do { \
- int rest = sizeof(buf) - 1 - strlen(buf); \
- if (rest > 0) { \
- int len = strlen(str); \
- if (len > rest) \
- len = rest; \
- strncat(buf, str, len); \
- buf[sizeof(buf) - 1] = '\0'; \
- } \
- } while (0);
struct xfrm_filter filter;
@@ -902,8 +891,8 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
prefix, title);
if (prefix)
- STRBUF_CAT(buf, prefix);
- STRBUF_CAT(buf, "\t");
+ strlcat(buf, prefix, sizeof(buf));
+ strlcat(buf, "\t", sizeof(buf));
fputs(buf, fp);
fprintf(fp, "replay-window %u ", xsinfo->replay_window);
@@ -944,7 +933,7 @@ void xfrm_state_info_print(struct xfrm_usersa_info *xsinfo,
char sbuf[STRBUF_SIZE];
memcpy(sbuf, buf, sizeof(sbuf));
- STRBUF_CAT(sbuf, "sel ");
+ strlcat(sbuf, "sel ", sizeof(sbuf));
xfrm_selector_print(&xsinfo->sel, xsinfo->family, fp, sbuf);
}
@@ -992,8 +981,8 @@ void xfrm_policy_info_print(struct xfrm_userpolicy_info *xpinfo,
}
if (prefix)
- STRBUF_CAT(buf, prefix);
- STRBUF_CAT(buf, "\t");
+ strlcat(buf, prefix, sizeof(buf));
+ strlcat(buf, "\t", sizeof(buf));
fputs(buf, fp);
if (xpinfo->dir >= XFRM_POLICY_MAX) {
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 2/6] Convert the obvious cases to strlcpy()
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
This converts the typical idiom of manually terminating the buffer after
a call to strncpy().
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
ip/ipnetns.c | 3 +--
ip/iproute_lwtunnel.c | 3 +--
ip/ipvrf.c | 3 +--
lib/bpf.c | 3 +--
lib/fs.c | 3 +--
lib/inet_proto.c | 3 +--
misc/ss.c | 3 +--
tc/em_ipset.c | 3 +--
8 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/ip/ipnetns.c b/ip/ipnetns.c
index 9ee1fe6a51f9c..afb4978a5be7d 100644
--- a/ip/ipnetns.c
+++ b/ip/ipnetns.c
@@ -518,8 +518,7 @@ int netns_identify_pid(const char *pidstr, char *name, int len)
if ((st.st_dev == netst.st_dev) &&
(st.st_ino == netst.st_ino)) {
- strncpy(name, entry->d_name, len - 1);
- name[len - 1] = '\0';
+ strlcpy(name, entry->d_name, len);
}
}
closedir(dir);
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index 1a3dc4d4c0ed9..4c2d3b07e3b5c 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -325,8 +325,7 @@ static int parse_encap_seg6(struct rtattr *rta, size_t len, int *argcp,
invarg("\"segs\" provided before \"mode\"\n",
*argv);
- strncpy(segbuf, *argv, 1024);
- segbuf[1023] = 0;
+ strlcpy(segbuf, *argv, 1024);
} else if (strcmp(*argv, "hmac") == 0) {
NEXT_ARG();
if (hmac_ok++)
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index e6fad32abd956..b74c501e0970c 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -336,8 +336,7 @@ static int vrf_path(char *vpath, size_t len)
if (vrf)
*vrf = '\0';
- strncpy(vpath, start, len - 1);
- vpath[len - 1] = '\0';
+ strlcpy(vpath, start, len);
/* if vrf path is just / then return nothing */
if (!strcmp(vpath, "/"))
diff --git a/lib/bpf.c b/lib/bpf.c
index 0bd0a95eafe6c..c180934acc7dc 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -512,8 +512,7 @@ static const char *bpf_find_mntpt_single(unsigned long magic, char *mnt,
ret = bpf_valid_mntpt(mntpt, magic);
if (!ret) {
- strncpy(mnt, mntpt, len - 1);
- mnt[len - 1] = 0;
+ strlcpy(mnt, mntpt, len);
return mnt;
}
diff --git a/lib/fs.c b/lib/fs.c
index ebe05cd44e11b..86efd4ed2ed80 100644
--- a/lib/fs.c
+++ b/lib/fs.c
@@ -172,8 +172,7 @@ int get_command_name(const char *pid, char *comm, size_t len)
if (nl)
*nl = '\0';
- strncpy(comm, name, len - 1);
- comm[len - 1] = '\0';
+ strlcpy(comm, name, len);
break;
}
diff --git a/lib/inet_proto.c b/lib/inet_proto.c
index 53c029039b6d5..bdfd52fdafe5a 100644
--- a/lib/inet_proto.c
+++ b/lib/inet_proto.c
@@ -38,8 +38,7 @@ const char *inet_proto_n2a(int proto, char *buf, int len)
free(ncache);
icache = proto;
ncache = strdup(pe->p_name);
- strncpy(buf, pe->p_name, len - 1);
- buf[len - 1] = '\0';
+ strlcpy(buf, pe->p_name, len);
return buf;
}
snprintf(buf, len, "ipproto-%d", proto);
diff --git a/misc/ss.c b/misc/ss.c
index 2c9e80e696595..dd8dfaa4e70db 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -425,8 +425,7 @@ static void user_ent_hash_build(void)
user_ent_hash_build_init = 1;
- strncpy(name, root, sizeof(name)-1);
- name[sizeof(name)-1] = 0;
+ strlcpy(name, root, sizeof(name));
if (strlen(name) == 0 || name[strlen(name)-1] != '/')
strcat(name, "/");
diff --git a/tc/em_ipset.c b/tc/em_ipset.c
index b59756515d239..48b287f5ba3b2 100644
--- a/tc/em_ipset.c
+++ b/tc/em_ipset.c
@@ -145,8 +145,7 @@ get_set_byname(const char *setname, struct xt_set_info *info)
int res;
req.op = IP_SET_OP_GET_BYNAME;
- strncpy(req.set.name, setname, IPSET_MAXNAMELEN);
- req.set.name[IPSET_MAXNAMELEN - 1] = '\0';
+ strlcpy(req.set.name, setname, IPSET_MAXNAMELEN);
res = do_getsockopt(&req);
if (res != 0)
return -1;
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 6/6] lnstat_util: Make sure buffer is NUL-terminated
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
Can't use strlcpy() here since lnstat is not linked against libutil.
While being at it, fix coding style in that chunk as well.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
misc/lnstat_util.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/misc/lnstat_util.c b/misc/lnstat_util.c
index ec19238c24b94..c2dc42ec1ff12 100644
--- a/misc/lnstat_util.c
+++ b/misc/lnstat_util.c
@@ -150,7 +150,8 @@ static int lnstat_scan_compat_rtstat_fields(struct lnstat_file *lf)
{
char buf[FGETS_BUF_SIZE];
- strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf)-1);
+ strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf) - 1);
+ buf[sizeof(buf) - 1] = '\0';
return __lnstat_scan_fields(lf, buf);
}
--
2.13.1
^ permalink raw reply related
* [iproute PATCH 5/6] tc_util: No need to terminate an snprintf'ed buffer
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
snprintf() won't leave the buffer unterminated, so manually terminating
is not necessary here.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tc/tc_util.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/tc/tc_util.c b/tc/tc_util.c
index 371046839ba9f..50d355046bdad 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -434,7 +434,6 @@ static const char *action_n2a(int action)
return "trap";
default:
snprintf(buf, 64, "%d", action);
- buf[63] = '\0';
return buf;
}
}
--
2.13.1
^ permalink raw reply related
* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-01 16:53 UTC (permalink / raw)
To: Jesus Sanchez-Palencia
Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
intel-wired-lan, andre.guedes, ivan.briano, boon.leong.ong
In-Reply-To: <1519366e-5418-4dda-db00-5bf50a1c67c4@intel.com>
On Fri, Sep 01, 2017 at 09:12:17AM -0700, Jesus Sanchez-Palencia wrote:
> Is it correct to assume you are referring to an interface for Launchtime here?
Yes.
Thanks,
Richard
^ permalink raw reply
* [iproute PATCH 3/6] Convert harmful calls to strncpy() to strlcpy()
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>
This patch converts spots where manual buffer termination was missing to
strlcpy() since that does what is needed.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
genl/ctrl.c | 2 +-
ip/ipvrf.c | 2 +-
ip/xfrm_state.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/genl/ctrl.c b/genl/ctrl.c
index 6abd52582d0d3..448988eb90e2b 100644
--- a/genl/ctrl.c
+++ b/genl/ctrl.c
@@ -313,7 +313,7 @@ static int ctrl_list(int cmd, int argc, char **argv)
if (matches(*argv, "name") == 0) {
NEXT_ARG();
- strncpy(d, *argv, sizeof (d) - 1);
+ strlcpy(d, *argv, sizeof(d));
addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME,
d, strlen(d) + 1);
} else if (matches(*argv, "id") == 0) {
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index b74c501e0970c..f9277e1e66bdf 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -74,7 +74,7 @@ static int vrf_identify(pid_t pid, char *name, size_t len)
if (end)
*end = '\0';
- strncpy(name, vrf, len - 1);
+ strlcpy(name, vrf, len);
break;
}
}
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index e11c93bf1c3b5..4483fb8f71d2c 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -125,7 +125,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
fprintf(stderr, "warning: ALGO-NAME/ALGO-KEYMAT values will be sent to the kernel promiscuously! (verifying them isn't implemented yet)\n");
#endif
- strncpy(alg->alg_name, name, sizeof(alg->alg_name));
+ strlcpy(alg->alg_name, name, sizeof(alg->alg_name));
if (slen > 2 && strncmp(key, "0x", 2) == 0) {
/* split two chars "0x" from the top */
--
2.13.1
^ permalink raw reply related
* [PATCH net-next 0/4] bpf: add two helpers to read perf event enabled/running time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch set implements two helper functions.
The helper bpf_perf_read_counter_time reads counter/time_enabled/time_running
for perf event array map. The helper bpf_perf_prog_read_time read
time_enabled/time_running for bpf prog with type BPF_PROG_TYPE_PERF_EVENT.
Yonghong Song (4):
bpf: add helper bpf_perf_read_counter_time for perf event array map
bpf: add a test case for helper bpf_perf_read_counter_time
bpf: add helper bpf_perf_prog_read_time
bpf: add a test case for helper bpf_perf_prog_read_time
include/linux/perf_event.h | 3 ++
include/uapi/linux/bpf.h | 29 +++++++++++-
kernel/bpf/verifier.c | 4 +-
kernel/events/core.c | 3 +-
kernel/trace/bpf_trace.c | 73 +++++++++++++++++++++++++++++--
samples/bpf/trace_event_kern.c | 5 +++
samples/bpf/tracex6_kern.c | 26 +++++++++++
samples/bpf/tracex6_user.c | 13 +++++-
tools/testing/selftests/bpf/bpf_helpers.h | 7 +++
9 files changed, 155 insertions(+), 8 deletions(-)
--
2.9.5
^ permalink raw reply
* [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>
Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
normalized_num_samples = num_samples * time_enabled / time_running
normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.
This patch adds helper bpf_perf_read_counter_time for kprobed based perf
event array map, to read perf counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
To achieve scaling factor between two bpf invocations, users
can can use cpu_id as the key (which is typical for perf array usage model)
to remember the previous value and do the calculation inside the
bpf program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 2 ++
include/uapi/linux/bpf.h | 21 +++++++++++++++++++-
kernel/bpf/verifier.c | 4 +++-
kernel/events/core.c | 2 +-
kernel/trace/bpf_trace.c | 49 ++++++++++++++++++++++++++++++++++++++++++----
5 files changed, 71 insertions(+), 7 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index b14095b..7fd5e94 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -901,6 +901,8 @@ extern void perf_pmu_migrate_context(struct pmu *pmu,
int perf_event_read_local(struct perf_event *event, u64 *value);
extern u64 perf_event_read_value(struct perf_event *event,
u64 *enabled, u64 *running);
+extern void calc_timer_values(struct perf_event *event, u64 *now,
+ u64 *enabled, u64 *running);
struct perf_sample_data {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba848b7..9c23bef 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -582,6 +582,14 @@ union bpf_attr {
* @map: pointer to sockmap to update
* @key: key to insert/update sock in map
* @flags: same flags as map update elem
+ *
+ * int bpf_perf_read_counter_time(map, flags, counter_time_buf, buf_size)
+ * read perf event counter value and perf event enabled/running time
+ * @map: pointer to perf_event_array map
+ * @flags: index of event in the map or bitmask flags
+ * @counter_time_buf: buf to fill
+ * @buf_size: size of the counter_time_buf
+ * Return: 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -638,6 +646,7 @@ union bpf_attr {
FN(redirect_map), \
FN(sk_redirect_map), \
FN(sock_map_update), \
+ FN(perf_read_counter_time), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -681,7 +690,8 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_read_counter_time flags. */
#define BPF_F_INDEX_MASK 0xffffffffULL
#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
/* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -864,4 +874,13 @@ enum {
#define TCP_BPF_IW 1001 /* Set TCP initial congestion window */
#define TCP_BPF_SNDCWND_CLAMP 1002 /* Set sndcwnd_clamp */
+struct bpf_perf_time {
+ __u64 enabled;
+ __u64 running;
+};
+struct bpf_perf_counter_time {
+ __u64 counter;
+ struct bpf_perf_time time;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..c4d29e3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1494,7 +1494,8 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
if (func_id != BPF_FUNC_perf_event_read &&
- func_id != BPF_FUNC_perf_event_output)
+ func_id != BPF_FUNC_perf_event_output &&
+ func_id != BPF_FUNC_perf_read_counter_time)
goto error;
break;
case BPF_MAP_TYPE_STACK_TRACE:
@@ -1537,6 +1538,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
break;
case BPF_FUNC_perf_event_read:
case BPF_FUNC_perf_event_output:
+ case BPF_FUNC_perf_read_counter_time:
if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
goto error;
break;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8c01572..ef5c7fb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4883,7 +4883,7 @@ static int perf_event_index(struct perf_event *event)
return event->pmu->event_idx(event);
}
-static void calc_timer_values(struct perf_event *event,
+void calc_timer_values(struct perf_event *event,
u64 *now,
u64 *enabled,
u64 *running)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dc498b6..b807b1a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
return &bpf_trace_printk_proto;
}
-BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
-{
+static __always_inline int
+get_map_perf_counter(struct bpf_map *map, u64 flags,
+ u64 *value, struct perf_event **pe) {
struct bpf_array *array = container_of(map, struct bpf_array, map);
unsigned int cpu = smp_processor_id();
u64 index = flags & BPF_F_INDEX_MASK;
struct bpf_event_entry *ee;
- u64 value = 0;
int err;
if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
@@ -275,7 +275,19 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
if (!ee)
return -ENOENT;
- err = perf_event_read_local(ee->event, &value);
+ err = perf_event_read_local(ee->event, value);
+ if (!err && pe)
+ *pe = ee->event;
+ return err;
+}
+
+
+BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
+{
+ u64 value = 0;
+ int err;
+
+ err = get_map_perf_counter(map, flags, &value, NULL);
/*
* this api is ugly since we miss [-22..-2] range of valid
* counter values, but that's uapi
@@ -285,6 +297,23 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
return value;
}
+BPF_CALL_4(bpf_perf_read_counter_time, struct bpf_map *, map, u64, flags,
+ struct bpf_perf_counter_time *, buf, u32, size)
+{
+ struct perf_event *pe;
+ u64 now;
+ int err;
+
+ if (unlikely(size != sizeof(struct bpf_perf_counter_time)))
+ return -EINVAL;
+ err = get_map_perf_counter(map, flags, &buf->counter, &pe);
+ if (err)
+ return err;
+
+ calc_timer_values(pe, &now, &buf->time.enabled, &buf->time.running);
+ return 0;
+}
+
static const struct bpf_func_proto bpf_perf_event_read_proto = {
.func = bpf_perf_event_read,
.gpl_only = true,
@@ -293,6 +322,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
.arg2_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_read_counter_time_proto = {
+ .func = bpf_perf_read_counter_time,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_CONST_MAP_PTR,
+ .arg2_type = ARG_ANYTHING,
+ .arg3_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg4_type = ARG_CONST_SIZE,
+};
+
static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
static __always_inline u64
@@ -499,6 +538,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
return &bpf_perf_event_output_proto;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto;
+ case BPF_FUNC_perf_read_counter_time:
+ return &bpf_perf_read_counter_time_proto;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH net-next 4/4] bpf: add a test case for helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>
The bpf sample program trace_event is enhanced to use the new
helper to print out enabled/running time.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/trace_event_kern.c | 5 +++++
tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
2 files changed, 8 insertions(+)
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 41b6115..f372660 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -37,8 +37,10 @@ struct bpf_map_def SEC("maps") stackmap = {
SEC("perf_event")
int bpf_prog1(struct bpf_perf_event_data *ctx)
{
+ char time_fmt[] = "Time Enabled: %lld, Time Running: %lld";
char fmt[] = "CPU-%d period %lld ip %llx";
u32 cpu = bpf_get_smp_processor_id();
+ struct bpf_perf_time time_buf;
struct key_t key;
u64 *val, one = 1;
@@ -54,6 +56,9 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
return 0;
}
+ bpf_perf_prog_read_time(ctx, (void *)&time_buf, sizeof(struct bpf_perf_time));
+ bpf_trace_printk(time_fmt, sizeof(time_fmt), time_buf.enabled, time_buf.running);
+
val = bpf_map_lookup_elem(&counts, &key);
if (val)
(*val)++;
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index fe41852..ddad690 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -74,6 +74,9 @@ static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
void *counter_time_buf,
unsigned int buf_size) =
(void *) BPF_FUNC_perf_read_counter_time;
+static int (*bpf_perf_prog_read_time)(void *ctx, void *time_buf,
+ unsigned int size) =
+ (void *) BPF_FUNC_perf_prog_read_time;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* [PATCH net-next 3/4] bpf: add helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>
This patch adds helper bpf_perf_prog_read_time for perf event based bpf
programs, to read event enabled/running time.
The enabled/running time is accumulated since the perf event open.
The typical use case for perf event based bpf program is to attach itself
to a single event. In such cases, if it is desirable to get scaling factor
between two bpf invocations, users can can save the time values in a map,
and use the value from the map and the current value to calculate
the scaling factor.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/perf_event.h | 1 +
include/uapi/linux/bpf.h | 8 ++++++++
kernel/events/core.c | 1 +
kernel/trace/bpf_trace.c | 24 ++++++++++++++++++++++++
4 files changed, 34 insertions(+)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7fd5e94..92955fc 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -821,6 +821,7 @@ struct perf_output_handle {
struct bpf_perf_event_data_kern {
struct pt_regs *regs;
struct perf_sample_data *data;
+ struct perf_event *event;
};
#ifdef CONFIG_CGROUP_PERF
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9c23bef..1ae55c8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -590,6 +590,13 @@ union bpf_attr {
* @counter_time_buf: buf to fill
* @buf_size: size of the counter_time_buf
* Return: 0 on success or negative error code
+ *
+ * int bpf_perf_prog_read_time(ctx, time_buf, buf_size)
+ * Read perf event enabled and running time
+ * @ctx: pointer to ctx
+ * @time_buf: buf to fill
+ * @buf_size: size of the time_buf
+ * Return : 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -647,6 +654,7 @@ union bpf_attr {
FN(sk_redirect_map), \
FN(sock_map_update), \
FN(perf_read_counter_time), \
+ FN(perf_prog_read_time), \
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ef5c7fb..1f16f1f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8019,6 +8019,7 @@ static void bpf_overflow_handler(struct perf_event *event,
struct bpf_perf_event_data_kern ctx = {
.data = data,
.regs = regs,
+ .event = event,
};
int ret = 0;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index b807b1a..e97620a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -608,6 +608,19 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
flags, 0, 0);
}
+BPF_CALL_3(bpf_perf_prog_read_time_tp, void *, ctx, struct bpf_perf_time *,
+ time_buf, u32, size)
+{
+ struct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;
+ u64 now;
+
+ if (size != sizeof(struct bpf_perf_time))
+ return -EINVAL;
+
+ calc_timer_values(kctx->event, &now, &time_buf->enabled, &time_buf->running);
+ return 0;
+}
+
static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.func = bpf_get_stackid_tp,
.gpl_only = true,
@@ -617,6 +630,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
.arg3_type = ARG_ANYTHING,
};
+static const struct bpf_func_proto bpf_perf_prog_read_time_proto_tp = {
+ .func = bpf_perf_prog_read_time_tp,
+ .gpl_only = true,
+ .ret_type = RET_INTEGER,
+ .arg1_type = ARG_PTR_TO_CTX,
+ .arg2_type = ARG_PTR_TO_UNINIT_MEM,
+ .arg3_type = ARG_CONST_SIZE,
+};
+
static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
{
switch (func_id) {
@@ -624,6 +646,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
return &bpf_perf_event_output_proto_tp;
case BPF_FUNC_get_stackid:
return &bpf_get_stackid_proto_tp;
+ case BPF_FUNC_perf_prog_read_time:
+ return &bpf_perf_prog_read_time_proto_tp;
default:
return tracing_func_proto(func_id);
}
--
2.9.5
^ permalink raw reply related
* [PATCH net-next 2/4] bpf: add a test case for helper bpf_perf_read_counter_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>
The bpf sample program tracex6 is enhanced to use the new
helper to read enabled/running time.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/tracex6_kern.c | 26 ++++++++++++++++++++++++++
samples/bpf/tracex6_user.c | 13 ++++++++++++-
tools/testing/selftests/bpf/bpf_helpers.h | 4 ++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/samples/bpf/tracex6_kern.c b/samples/bpf/tracex6_kern.c
index e7d1803..46acfef 100644
--- a/samples/bpf/tracex6_kern.c
+++ b/samples/bpf/tracex6_kern.c
@@ -15,6 +15,12 @@ struct bpf_map_def SEC("maps") values = {
.value_size = sizeof(u64),
.max_entries = 64,
};
+struct bpf_map_def SEC("maps") values2 = {
+ .type = BPF_MAP_TYPE_HASH,
+ .key_size = sizeof(int),
+ .value_size = sizeof(struct bpf_perf_counter_time),
+ .max_entries = 64,
+};
SEC("kprobe/htab_map_get_next_key")
int bpf_prog1(struct pt_regs *ctx)
@@ -37,5 +43,25 @@ int bpf_prog1(struct pt_regs *ctx)
return 0;
}
+SEC("kprobe/htab_map_lookup_elem")
+int bpf_prog2(struct pt_regs *ctx)
+{
+ u32 key = bpf_get_smp_processor_id();
+ struct bpf_perf_counter_time *val, buf;
+ int error;
+
+ error = bpf_perf_read_counter_time(&counters, key, &buf, sizeof(buf));
+ if (error)
+ return 0;
+
+ val = bpf_map_lookup_elem(&values2, &key);
+ if (val)
+ *val = buf;
+ else
+ bpf_map_update_elem(&values2, &key, &buf, BPF_NOEXIST);
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/tracex6_user.c b/samples/bpf/tracex6_user.c
index a05a99a..2a0c5d8 100644
--- a/samples/bpf/tracex6_user.c
+++ b/samples/bpf/tracex6_user.c
@@ -22,6 +22,7 @@
static void check_on_cpu(int cpu, struct perf_event_attr *attr)
{
+ struct bpf_perf_counter_time value2;
int pmu_fd, error = 0;
cpu_set_t set;
__u64 value;
@@ -46,8 +47,18 @@ static void check_on_cpu(int cpu, struct perf_event_attr *attr)
fprintf(stderr, "Value missing for CPU %d\n", cpu);
error = 1;
goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: %llu\n", cpu, value);
+ }
+ /* The above bpf_map_lookup_elem should trigger the second kprobe */
+ if (bpf_map_lookup_elem(map_fd[2], &cpu, &value2)) {
+ fprintf(stderr, "Value2 missing for CPU %d\n", cpu);
+ error = 1;
+ goto on_exit;
+ } else {
+ fprintf(stderr, "CPU %d: counter: %llu, enabled: %llu, running: %llu\n", cpu,
+ value2.counter, value2.time.enabled, value2.time.running);
}
- fprintf(stderr, "CPU %d: %llu\n", cpu, value);
on_exit:
assert(bpf_map_delete_elem(map_fd[0], &cpu) == 0 || error);
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 36fb916..fe41852 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -70,6 +70,10 @@ static int (*bpf_sk_redirect_map)(void *map, int key, int flags) =
static int (*bpf_sock_map_update)(void *map, void *key, void *value,
unsigned long long flags) =
(void *) BPF_FUNC_sock_map_update;
+static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
+ void *counter_time_buf,
+ unsigned int buf_size) =
+ (void *) BPF_FUNC_perf_read_counter_time;
/* llvm builtin functions that eBPF C program may use to
--
2.9.5
^ permalink raw reply related
* Re: pull request (net-next): ipsec-next 2017-09-01
From: David Miller @ 2017-09-01 16:54 UTC (permalink / raw)
To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <1504251046-17106-1-git-send-email-steffen.klassert@secunet.com>
From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 1 Sep 2017 09:30:44 +0200
> This should be the last ipsec-next pull request for this
> release cycle:
>
> 1) Support netdevice ESP trailer removal when decryption
> is offloaded. From Yossi Kuperman.
>
> 2) Fix overwritten return value of copy_sec_ctx().
>
> Please pull or let me know if there are problems.
Pulled, thanks Steffen.
^ permalink raw reply
* Re: [iproute PATCH 50/51] Check user supplied interface name lengths
From: Phil Sutter @ 2017-09-01 16:56 UTC (permalink / raw)
To: Stephen Hemminger, netdev
In-Reply-To: <20170815165132.GI16375@orbyte.nwl.cc>
Hi Stephen,
On Tue, Aug 15, 2017 at 06:51:32PM +0200, Phil Sutter wrote:
> On Tue, Aug 15, 2017 at 09:09:45AM -0700, Stephen Hemminger wrote:
> > On Sat, 12 Aug 2017 14:05:09 +0200
> > Phil Sutter <phil@nwl.cc> wrote:
> >
> > > +void assert_valid_dev_name(const char *, const char *);
> >
> > Not a fan of long function names.
> > “I have only made this letter longer because I have not had the time to make it shorter."
>
> :)
>
> > Maybe just add a new function addattr_ifname() and add the checking there?
>
> It is not only about netlink attributes - these are in fact
> unproblematic, since they allow for interface names longer than
> IFNAMSIZ-1 and the kernel will then reject. The length check has to
> happen before copying into any IFNAMSIZ-sized buffer takes place (e.g.
> ifr_name of struct ifreq). Logically I would prefer to perform the
> checks right at the point user input parsing takes place since it
> belongs there.
>
> I could rename the function to 'ifname_valid' instead? I liked having
> 'assert' in the name though, since the function calls exit() on error.
Any thoughts on this?
Cheers, Phil
^ permalink raw reply
* Re: [PATCH net-next 0/3] bpf: Improve LRU map lookup performance
From: David Miller @ 2017-09-01 16:57 UTC (permalink / raw)
To: kafai; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <20170901062713.1842249-1-kafai@fb.com>
From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 31 Aug 2017 23:27:10 -0700
> This patchset improves the lookup performance of the LRU map.
> Please see individual patch for details.
Series applied, thanks Martin.
^ permalink raw reply
* Re: [patch net] mlxsw: spectrum: Forbid linking to devices that have uppers
From: David Miller @ 2017-09-01 17:00 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, nogahf, mlxsw
In-Reply-To: <20170901085231.1396-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 1 Sep 2017 10:52:31 +0200
> From: Ido Schimmel <idosch@mellanox.com>
>
> The mlxsw driver relies on NETDEV_CHANGEUPPER events to configure the
> device in case a port is enslaved to a master netdev such as bridge or
> bond.
>
> Since the driver ignores events unrelated to its ports and their
> uppers, it's possible to engineer situations in which the device's data
> path differs from the kernel's.
>
> One example to such a situation is when a port is enslaved to a bond
> that is already enslaved to a bridge. When the bond was enslaved the
> driver ignored the event - as the bond wasn't one of its uppers - and
> therefore a bridge port instance isn't created in the device.
>
> Until such configurations are supported forbid them by checking that the
> upper device doesn't have uppers of its own.
>
> Fixes: 0d65fc13042f ("mlxsw: spectrum: Implement LAG port join/leave")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: Nogah Frankel <nogahf@mellanox.com>
> Tested-by: Nogah Frankel <nogahf@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [patch net-next 0/2] mlxsw: spectrum_router: Couple of fixes
From: David Miller @ 2017-09-01 17:01 UTC (permalink / raw)
To: jiri; +Cc: netdev, idosch, mlxsw
In-Reply-To: <20170901085856.2286-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Fri, 1 Sep 2017 10:58:54 +0200
> From: Jiri Pirko <jiri@mellanox.com>
>
> Ido Schimmel (2):
> mlxsw: spectrum_router: Trap packets hitting anycast routes
> mlxsw: spectrum_router: Set abort trap in all virtual routers
Series applied.
^ permalink raw reply
* Re: [PATCH net-next 2/2] flow_dissector: Add limits for encapsulation and EH
From: Hannes Frederic Sowa @ 2017-09-01 17:05 UTC (permalink / raw)
To: Tom Herbert
Cc: Tom Herbert, David S . Miller, Linux Kernel Network Developers,
alex.popov
In-Reply-To: <CALx6S37304j28=TdDYaj8dS6G=bhjzMkaVsieCcK3hLWNSVKzA@mail.gmail.com>
Tom Herbert <tom@herbertland.com> writes:
> On Fri, Sep 1, 2017 at 9:35 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> Hello Tom,
>>
>> Tom Herbert <tom@quantonium.net> writes:
>>
>>> On Fri, Sep 1, 2017 at 6:32 AM, Hannes Frederic Sowa
>>> <hannes@stressinduktion.org> wrote:
>>>> Tom Herbert <tom@quantonium.net> writes:
>>>>
>>>>> In flow dissector there are no limits to the number of nested
>>>>> encapsulations that might be dissected which makes for a nice DOS
>>>>> attack. This patch limits for dissecting nested encapsulations
>>>>> as well as for dissecting over extension headers.
>>>>
>>>> I was actually more referring to your patch, because the flow dissector
>>>> right now is not stack recursive. Your changes would make it doing
>>>> recursion on the stack.
>>>
>>> I don't believe those patches had any recursion.
>>
>> I was wrong with stack recursion, you handle that using the
>> FLOW_DISSECT_RET_PROTO_AGAIN return value thus leaving the stack frame
>> again, sorry.
>>
>> But otherwise the walk would be unlimited (based on the packet size) in
>> your first patchset, correct? See this malicious example:
>>
>> | IP1 | UDP1 | VXLAN1 | Ethernet | IP2 | UDP2 | VXLAN2 | ...
>>
> Without the limits patch I subsequently proposed, yes. However, this
> is true for all the other encapsulations anyway; there's is nothing
> unique about UDP encapsulations in this regard (hence with the limit
> patch should generally apply to all encapsulations).
I used this example to show its possible security implications. Other
encaps definitely have the same problems.
>> where IP1 == IP2, UDP1 == UDP2 and VXLAN1 != VXLAN2?
>>
>> Notice that because IP1 == IP2 and UDP1 == UDP2 it seems to me it would
>> hit the same socket again. We would be prone to overwrite vxlan id 1
>> with vxlan id 2 in the key thus the key would be malicious and traffic
>> could be injected into other tenant networks, if the encapsulated
>> packets within VXLAN1 could be generated by a malicious user?
>>
> This is why flow dissection is not an authoritative parsing of the
> packet. It can be wrong or misleading because it doesn't have all the
> context, doesn't necessarily parse the whole chain, and only returns
> one set of information (for instance only one pair of IP addresses
> when there may be more in a packet). It's just a best effort mechanism
> that is great for computing a hash for instance. If someone is
> steering a packet to a VM based on the output of flow dissector that
> is a bug; the only correct way to do this is to go through the normal
> receive protocol processing path.
I think it must be agreed upon what flow dissector is. Especially I am
concerned about its use in cls_flower (or vice versa this patch).
>> I was actually not concerned about the "recursion" but merely about
>> updating the values to the innermost values.
>>
> See my previous comment about use STOP_AT_ENCAP.
For an authorative parser, for which it gets used in flower right now
STOP_AT_ENCAP might not make too much sense, because it might want to
look at one additional level of encapsulation information. But if you
don't consider the dissector to be an authorative parser for packets, it
would be okay.
Btw., I fear this recursion problem exists right now also with flower's
use of lwt.
[...]
Thanks,
Hannes
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net: mvpp2: optional PHYs and GoP link irq
From: David Miller @ 2017-09-01 17:09 UTC (permalink / raw)
To: antoine.tenart
Cc: andrew, gregory.clement, thomas.petazzoni, nadavh, linux,
linux-kernel, mw, stefanc, miquel.raynal, netdev
In-Reply-To: <20170901090455.32316-1-antoine.tenart@free-electrons.com>
From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Fri, 1 Sep 2017 11:04:51 +0200
> This series aims at making the driver work when no PHY is connected
> between a port and the physical layer and not described as a fixed-phy.
> This is useful for some usecases such as when a switch is connected
> directly to the serdes lanes. It can also be used for SFP ports on the
> 7k-db and 8k-db while waiting for the phylink support to land in (which
> should be part of another series).
>
> This series makes the phy optional in the PPv2 driver, and then adds
> the support for the GoP port link interrupt to handle link status
> changes on such ports.
>
> This was tested using the SFP ports on the 7k-db and 8k-db boards.
Series applied.
^ 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