* Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup
From: Rob Herring @ 2017-09-01 21:26 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Bjorn Andersson, Gustavo F. Padovan, Johan Hedberg,
David S. Miller, open list:BLUETOOTH DRIVERS, Network Development,
LKML, linux-arm-msm, Loic Poulain
In-Reply-To: <5EC41247-54CD-4F1D-84EC-E94F80A67805@holtmann.org>
On Fri, Sep 1, 2017 at 3:47 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Bjorn,
>
>> Bluetooth BD address can be retrieved in the same way as
>> for wcnss-wlan MAC address. This patch mainly stores the
>> local-mac-address property and sets the BD address during
>> hci device setup.
>>
>> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
>> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>> ---
>> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++
>> 1 file changed, 28 insertions(+)
>>
>> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
>> index d00c4fdae924..443bb2099329 100644
>> --- a/drivers/bluetooth/btqcomsmd.c
>> +++ b/drivers/bluetooth/btqcomsmd.c
>> @@ -26,6 +26,7 @@
>> struct btqcomsmd {
>> struct hci_dev *hdev;
>>
>> + const bdaddr_t *addr;
>> struct rpmsg_endpoint *acl_channel;
>> struct rpmsg_endpoint *cmd_channel;
>> };
>> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)
>> return 0;
>> }
>>
>> +static int btqcomsmd_setup(struct hci_dev *hdev)
>> +{
>> + struct btqcomsmd *btq = hci_get_drvdata(hdev);
>> + struct sk_buff *skb;
>> +
>> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
>> + if (IS_ERR(skb))
>> + return PTR_ERR(skb);
>> + kfree_skb(skb);
>> +
>> + if (btq->addr) {
>> + bdaddr_t bdaddr;
>> +
>> + /* btq->addr stored with most significant byte first */
>> + baswap(&bdaddr, btq->addr);
>> + return qca_set_bdaddr_rome(hdev, &bdaddr);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int btqcomsmd_probe(struct platform_device *pdev)
>> {
>> struct btqcomsmd *btq;
>> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)
>> if (IS_ERR(btq->cmd_channel))
>> return PTR_ERR(btq->cmd_channel);
>>
>> + btq->addr = of_get_property(pdev->dev.of_node, "local-mac-address",
>> + &ret);
>> + if (ret != sizeof(bdaddr_t))
>> + btq->addr = NULL;
>> +
>> hdev = hci_alloc_dev();
>> if (!hdev)
>> return -ENOMEM;
>> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)
>> hdev->open = btqcomsmd_open;
>> hdev->close = btqcomsmd_close;
>> hdev->send = btqcomsmd_send;
>> + hdev->setup = btqcomsmd_setup;
>> hdev->set_bdaddr = qca_set_bdaddr_rome;
>
> I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.
>
> Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.
Use of "local-mac-address" for ethernet at least has existed as long
at OpenFirmware I think. For some platforms, DT is the only OTP. And
sometimes, the bootloader (like u-boot) stores MAC addresses and then
populates them on boot.
Seems like if we just let userspace deal with it, then we're back to a
btattach tool with every platform's specific way of reading the MAC
address.
Rob
^ permalink raw reply
* Re: [PATCH iproute2 0/2] fix "ip link show dev ..." for NICs with many VFs
From: Stephen Hemminger @ 2017-09-01 21:16 UTC (permalink / raw)
To: Michal Kubecek; +Cc: netdev, linux-kernel
In-Reply-To: <cover.1504283683.git.mkubecek@suse.cz>
On Fri, 1 Sep 2017 18:39:06 +0200 (CEST)
Michal Kubecek <mkubecek@suse.cz> wrote:
> 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(-)
>
Looks like the best set of solutions to the kernel side API issue.
Applied, thanks Michal.
^ permalink raw reply
* [PATCH v2 net-next 2/2] flow_dissector: Add limit for number of headers to dissect
From: Tom Herbert @ 2017-09-01 21:04 UTC (permalink / raw)
To: davem; +Cc: netdev, hannes, alex.popov, Tom Herbert
In-Reply-To: <20170901210412.2915-1-tom@quantonium.net>
In flow dissector there are no limits to the number of nested
encapsulations or headers that might be dissected which makes for a
nice DOS attack. This patch sets a limit of the number of headers
that flow dissector will parse.
Headers includes network layer headers, transport layer headers, shim
headers for encapsulation, IPv6 extension headers, etc. The limit for
maximum number of headers to parse has be set to fifteen to account for
a reasonable number of encapsulations, extension headers, VLAN,
in a packet. Note that this limit does not supercede the STOP_AT_*
flags which may stop processing before the headers limit is reached.
Reported-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Tom Herbert <tom@quantonium.net>
---
net/core/flow_dissector.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index e0ea17d1c7fc..0a977373d003 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -396,6 +396,18 @@ __skb_flow_dissect_ipv6(const struct sk_buff *skb,
key_ip->ttl = iph->hop_limit;
}
+/* Maximum number of protocol headers that can be parsed in
+ * __skb_flow_dissect
+ */
+#define MAX_FLOW_DISSECT_HDRS 15
+
+static bool skb_flow_dissect_allowed(int *num_hdrs)
+{
+ ++*num_hdrs;
+
+ return (*num_hdrs <= MAX_FLOW_DISSECT_HDRS);
+}
+
/**
* __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
@@ -427,6 +439,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector_key_vlan *key_vlan;
enum flow_dissect_ret fdret;
bool skip_vlan = false;
+ int num_hdrs = 0;
u8 ip_proto = 0;
bool ret;
@@ -714,7 +727,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_allowed(&num_hdrs))
+ goto proto_again;
+ goto out_good;
case FLOW_DISSECT_RET_CONTINUE:
case FLOW_DISSECT_RET_IPPROTO_AGAIN:
break;
@@ -843,9 +858,13 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
/* Process result of IP proto processing */
switch (fdret) {
case FLOW_DISSECT_RET_PROTO_AGAIN:
- goto proto_again;
+ if (skb_flow_dissect_allowed(&num_hdrs))
+ goto proto_again;
+ break;
case FLOW_DISSECT_RET_IPPROTO_AGAIN:
- goto ip_proto_again;
+ if (skb_flow_dissect_allowed(&num_hdrs))
+ goto ip_proto_again;
+ break;
case FLOW_DISSECT_RET_OUT_GOOD:
case FLOW_DISSECT_RET_CONTINUE:
break;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 1/2] flow_dissector: Cleanup control flow
From: Tom Herbert @ 2017-09-01 21:04 UTC (permalink / raw)
To: davem; +Cc: netdev, hannes, alex.popov, Tom Herbert
In-Reply-To: <20170901210412.2915-1-tom@quantonium.net>
__skb_flow_dissect is riddled with gotos that make discerning the flow,
debugging, and extending the capability difficult. This patch
reorganizes things so that we only perform goto's after the two main
switch statements (no gotos within the cases now). It also eliminates
several goto labels so that there are only two labels that can be target
for goto.
Reported-by: Alexander Popov <alex.popov@linux.com>
Signed-off-by: Tom Herbert <tom@quantonium.net>
---
include/net/flow_dissector.h | 8 ++
net/core/flow_dissector.c | 223 ++++++++++++++++++++++++++++---------------
2 files changed, 153 insertions(+), 78 deletions(-)
diff --git a/include/net/flow_dissector.h b/include/net/flow_dissector.h
index e2663e900b0a..fc3dce730a6b 100644
--- a/include/net/flow_dissector.h
+++ b/include/net/flow_dissector.h
@@ -19,6 +19,14 @@ struct flow_dissector_key_control {
#define FLOW_DIS_FIRST_FRAG BIT(1)
#define FLOW_DIS_ENCAPSULATION BIT(2)
+enum flow_dissect_ret {
+ FLOW_DISSECT_RET_OUT_GOOD,
+ FLOW_DISSECT_RET_OUT_BAD,
+ FLOW_DISSECT_RET_PROTO_AGAIN,
+ FLOW_DISSECT_RET_IPPROTO_AGAIN,
+ FLOW_DISSECT_RET_CONTINUE,
+};
+
/**
* struct flow_dissector_key_basic:
* @thoff: Transport header offset
diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index e2eaa1ff948d..e0ea17d1c7fc 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -115,12 +115,6 @@ __be32 __skb_flow_get_ports(const struct sk_buff *skb, int thoff, u8 ip_proto,
}
EXPORT_SYMBOL(__skb_flow_get_ports);
-enum flow_dissect_ret {
- FLOW_DISSECT_RET_OUT_GOOD,
- FLOW_DISSECT_RET_OUT_BAD,
- FLOW_DISSECT_RET_OUT_PROTO_AGAIN,
-};
-
static enum flow_dissect_ret
__skb_flow_dissect_mpls(const struct sk_buff *skb,
struct flow_dissector *flow_dissector,
@@ -341,7 +335,7 @@ __skb_flow_dissect_gre(const struct sk_buff *skb,
if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
return FLOW_DISSECT_RET_OUT_GOOD;
- return FLOW_DISSECT_RET_OUT_PROTO_AGAIN;
+ return FLOW_DISSECT_RET_PROTO_AGAIN;
}
static void
@@ -431,6 +425,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
struct flow_dissector_key_icmp *key_icmp;
struct flow_dissector_key_tags *key_tags;
struct flow_dissector_key_vlan *key_vlan;
+ enum flow_dissect_ret fdret;
bool skip_vlan = false;
u8 ip_proto = 0;
bool ret;
@@ -482,14 +477,19 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
}
proto_again:
+ fdret = FLOW_DISSECT_RET_CONTINUE;
+
switch (proto) {
case htons(ETH_P_IP): {
const struct iphdr *iph;
struct iphdr _iph;
-ip:
+
iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
- if (!iph || iph->ihl < 5)
- goto out_bad;
+ if (!iph || iph->ihl < 5) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
nhoff += iph->ihl * 4;
ip_proto = iph->protocol;
@@ -509,19 +509,25 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
key_control->flags |= FLOW_DIS_IS_FRAGMENT;
if (iph->frag_off & htons(IP_OFFSET)) {
- goto out_good;
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
} else {
key_control->flags |= FLOW_DIS_FIRST_FRAG;
- if (!(flags & FLOW_DISSECTOR_F_PARSE_1ST_FRAG))
- goto out_good;
+ if (!(flags &
+ FLOW_DISSECTOR_F_PARSE_1ST_FRAG)) {
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
+ }
}
}
__skb_flow_dissect_ipv4(skb, flow_dissector,
target_container, data, iph);
- if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
- goto out_good;
+ if (flags & FLOW_DISSECTOR_F_STOP_AT_L3) {
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
+ }
break;
}
@@ -529,10 +535,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
const struct ipv6hdr *iph;
struct ipv6hdr _iph;
-ipv6:
iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
- if (!iph)
- goto out_bad;
+ if (!iph) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
ip_proto = iph->nexthdr;
nhoff += sizeof(struct ipv6hdr);
@@ -561,15 +568,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
target_container);
key_tags->flow_label = ntohl(flow_label);
}
- if (flags & FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL)
- goto out_good;
+ if (flags & FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL) {
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
+ }
}
__skb_flow_dissect_ipv6(skb, flow_dissector,
target_container, data, iph);
if (flags & FLOW_DISSECTOR_F_STOP_AT_L3)
- goto out_good;
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
break;
}
@@ -585,12 +594,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
if (!vlan_tag_present || eth_type_vlan(skb->protocol)) {
vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
data, hlen, &_vlan);
- if (!vlan)
- goto out_bad;
+ if (!vlan) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
proto = vlan->h_vlan_encapsulated_proto;
nhoff += sizeof(*vlan);
- if (skip_vlan)
- goto proto_again;
+ if (skip_vlan) {
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
+ }
}
skip_vlan = true;
@@ -613,7 +627,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
}
}
- goto proto_again;
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
}
case htons(ETH_P_PPP_SES): {
struct {
@@ -621,18 +636,27 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
__be16 proto;
} *hdr, _hdr;
hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
- if (!hdr)
- goto out_bad;
+ if (!hdr) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
proto = hdr->proto;
nhoff += PPPOE_SES_HLEN;
switch (proto) {
case htons(PPP_IP):
- goto ip;
+ proto = htons(ETH_P_IP);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
case htons(PPP_IPV6):
- goto ipv6;
+ proto = htons(ETH_P_IPV6);
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
default:
- goto out_bad;
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
}
+ break;
}
case htons(ETH_P_TIPC): {
struct {
@@ -640,8 +664,10 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
__be32 srcnode;
} *hdr, _hdr;
hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
- if (!hdr)
- goto out_bad;
+ if (!hdr) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
if (dissector_uses_key(flow_dissector,
FLOW_DISSECTOR_KEY_TIPC_ADDRS)) {
@@ -651,56 +677,62 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
key_addrs->tipcaddrs.srcnode = hdr->srcnode;
key_control->addr_type = FLOW_DISSECTOR_KEY_TIPC_ADDRS;
}
- goto out_good;
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
}
case htons(ETH_P_MPLS_UC):
case htons(ETH_P_MPLS_MC):
-mpls:
- switch (__skb_flow_dissect_mpls(skb, flow_dissector,
+ fdret = __skb_flow_dissect_mpls(skb, flow_dissector,
target_container, data,
- nhoff, hlen)) {
- case FLOW_DISSECT_RET_OUT_GOOD:
- goto out_good;
- case FLOW_DISSECT_RET_OUT_BAD:
- default:
- goto out_bad;
- }
+ nhoff, hlen);
+ break;
case htons(ETH_P_FCOE):
- if ((hlen - nhoff) < FCOE_HEADER_LEN)
- goto out_bad;
+ if ((hlen - nhoff) < FCOE_HEADER_LEN) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
nhoff += FCOE_HEADER_LEN;
- goto out_good;
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
case htons(ETH_P_ARP):
case htons(ETH_P_RARP):
- switch (__skb_flow_dissect_arp(skb, flow_dissector,
+ fdret = __skb_flow_dissect_arp(skb, flow_dissector,
target_container, data,
- nhoff, hlen)) {
- case FLOW_DISSECT_RET_OUT_GOOD:
- goto out_good;
- case FLOW_DISSECT_RET_OUT_BAD:
- default:
- goto out_bad;
- }
+ nhoff, hlen);
+ break;
+
+ default:
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
+
+ /* Process result of proto processing */
+ switch (fdret) {
+ case FLOW_DISSECT_RET_OUT_GOOD:
+ goto out_good;
+ case FLOW_DISSECT_RET_PROTO_AGAIN:
+ goto proto_again;
+ case FLOW_DISSECT_RET_CONTINUE:
+ case FLOW_DISSECT_RET_IPPROTO_AGAIN:
+ break;
+ case FLOW_DISSECT_RET_OUT_BAD:
default:
goto out_bad;
}
ip_proto_again:
+ fdret = FLOW_DISSECT_RET_CONTINUE;
+
switch (ip_proto) {
case IPPROTO_GRE:
- switch (__skb_flow_dissect_gre(skb, key_control, flow_dissector,
+ fdret = __skb_flow_dissect_gre(skb, key_control, flow_dissector,
target_container, data,
- &proto, &nhoff, &hlen, flags)) {
- case FLOW_DISSECT_RET_OUT_GOOD:
- goto out_good;
- case FLOW_DISSECT_RET_OUT_BAD:
- goto out_bad;
- case FLOW_DISSECT_RET_OUT_PROTO_AGAIN:
- goto proto_again;
- }
+ &proto, &nhoff, &hlen, flags);
+ break;
+
case NEXTHDR_HOP:
case NEXTHDR_ROUTING:
case NEXTHDR_DEST: {
@@ -711,13 +743,16 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
opthdr = __skb_header_pointer(skb, nhoff, sizeof(_opthdr),
data, hlen, &_opthdr);
- if (!opthdr)
- goto out_bad;
+ if (!opthdr) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
ip_proto = opthdr[0];
nhoff += (opthdr[1] + 1) << 3;
- goto ip_proto_again;
+ fdret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+ break;
}
case NEXTHDR_FRAGMENT: {
struct frag_hdr _fh, *fh;
@@ -728,8 +763,10 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
fh = __skb_header_pointer(skb, nhoff, sizeof(_fh),
data, hlen, &_fh);
- if (!fh)
- goto out_bad;
+ if (!fh) {
+ fdret = FLOW_DISSECT_RET_OUT_BAD;
+ break;
+ }
key_control->flags |= FLOW_DIS_IS_FRAGMENT;
@@ -738,34 +775,50 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
if (!(fh->frag_off & htons(IP6_OFFSET))) {
key_control->flags |= FLOW_DIS_FIRST_FRAG;
- if (flags & FLOW_DISSECTOR_F_PARSE_1ST_FRAG)
- goto ip_proto_again;
+ if (flags & FLOW_DISSECTOR_F_PARSE_1ST_FRAG) {
+ fdret = FLOW_DISSECT_RET_IPPROTO_AGAIN;
+ break;
+ }
}
- goto out_good;
+
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
}
case IPPROTO_IPIP:
proto = htons(ETH_P_IP);
key_control->flags |= FLOW_DIS_ENCAPSULATION;
- if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
- goto out_good;
+ if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) {
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
+ }
+
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
- goto ip;
case IPPROTO_IPV6:
proto = htons(ETH_P_IPV6);
key_control->flags |= FLOW_DIS_ENCAPSULATION;
- if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP)
- goto out_good;
+ if (flags & FLOW_DISSECTOR_F_STOP_AT_ENCAP) {
+ fdret = FLOW_DISSECT_RET_OUT_GOOD;
+ break;
+ }
+
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
+
- goto ipv6;
case IPPROTO_MPLS:
proto = htons(ETH_P_MPLS_UC);
- goto mpls;
+ fdret = FLOW_DISSECT_RET_PROTO_AGAIN;
+ break;
+
case IPPROTO_TCP:
__skb_flow_dissect_tcp(skb, flow_dissector, target_container,
data, nhoff, hlen);
break;
+
default:
break;
}
@@ -787,6 +840,20 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
key_icmp->icmp = skb_flow_get_be16(skb, nhoff, data, hlen);
}
+ /* Process result of IP proto processing */
+ switch (fdret) {
+ case FLOW_DISSECT_RET_PROTO_AGAIN:
+ goto proto_again;
+ case FLOW_DISSECT_RET_IPPROTO_AGAIN:
+ goto ip_proto_again;
+ case FLOW_DISSECT_RET_OUT_GOOD:
+ case FLOW_DISSECT_RET_CONTINUE:
+ break;
+ case FLOW_DISSECT_RET_OUT_BAD:
+ default:
+ goto out_bad;
+ }
+
out_good:
ret = true;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 net-next 0/2] flow_dissector: Flow dissector fixes
From: Tom Herbert @ 2017-09-01 21:04 UTC (permalink / raw)
To: davem; +Cc: netdev, hannes, alex.popov, Tom Herbert
This patch set fixes some basic issues with __skb_flow_dissect function.
Items addressed:
- Cleanup control flow in the function; in particular eliminate a
bunch of goto's and implement a simplified control flow model
- Add limits for number of encapsulations and headers that can be
dissected
v2:
- Simplify the logic for limits on flow dissection. Just set the
limit based on the number of headers the flow dissector can
processes. The accounted headers includes encapsulation headers,
extension headers, or other shim headers.
Tested:
Ran normal traffic, GUE, and VXLAN traffic.
*** BLURB HERE ***
Tom Herbert (2):
flow_dissector: Cleanup control flow
flow_dissector: Add limit for number of headers to dissect
include/net/flow_dissector.h | 8 ++
net/core/flow_dissector.c | 242 +++++++++++++++++++++++++++++--------------
2 files changed, 172 insertions(+), 78 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH net-next] inetpeer: fix RCU lookup()
From: Eric Dumazet @ 2017-09-01 21:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
Excess of seafood or something happened while I cooked the commit
adding RB tree to inetpeer.
Of course, RCU rules need to be respected or bad things can happen.
In this particular loop, we need to read *pp once per iteration, not
twice.
Fixes: b145425f269a ("inetpeer: remove AVL implementation in favor of RB tree")
Reported-by: John Sperbeck <jsperbeck@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/inetpeer.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 337ad41bb80a5fcd3db7ac674292c5b5d462982e..e7eb590c86ce2b33654c17c61619de74ff07bfd1 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -102,15 +102,18 @@ static struct inet_peer *lookup(const struct inetpeer_addr *daddr,
struct rb_node **parent_p,
struct rb_node ***pp_p)
{
- struct rb_node **pp, *parent;
+ struct rb_node **pp, *parent, *next;
struct inet_peer *p;
pp = &base->rb_root.rb_node;
parent = NULL;
- while (*pp) {
+ while (1) {
int cmp;
- parent = rcu_dereference_raw(*pp);
+ next = rcu_dereference_raw(*pp);
+ if (!next)
+ break;
+ parent = next;
p = rb_entry(parent, struct inet_peer, rb_node);
cmp = inetpeer_addr_cmp(daddr, &p->daddr);
if (cmp == 0) {
^ permalink raw reply related
* Re: [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Yonghong Song @ 2017-09-01 21:01 UTC (permalink / raw)
To: Peter Zijlstra, Alexei Starovoitov; +Cc: rostedt, daniel, netdev, kernel-team
In-Reply-To: <20170901205040.GU6524@worktop.programming.kicks-ass.net>
On 9/1/17 1:50 PM, Peter Zijlstra wrote:
> On Fri, Sep 01, 2017 at 01:29:17PM -0700, Alexei Starovoitov wrote:
>
>>> +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;
>>> +}
>>
>> Peter,
>> I believe we're doing it correctly above.
>> It's a copy paste of the same logic as in total_time_enabled/running.
>> We cannot expose total_time_enabled/running to bpf, since they are
>> different counters. The above two are specific to bpf usage.
>> See commit log.
>
> No, the patch is atrocious and the usage is wrong.
>
> Exporting a function called 'calc_timer_values' is a horrible violation
> of the namespace.
>
> And its wrong because it should be done in conjunction with
> perf_event_read_local(). You cannot afterwards call this because you
> don't know if the event was active when you read it and you don't have
> temporal guarantees; that is, reading these timestamps long after or
> before the read is wrong, and this interface allows it.
Thanks for explanation. Will push the read/calculate time
enabled/running inside the perf_event_read_local then.
>
> So no, sorry this is just fail.
>
^ permalink raw reply
* Re: [PATCH 1/2] Bluetooth: make baswap src const
From: Marcel Holtmann @ 2017-09-01 20:51 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
linux-bluetooth, netdev, linux-kernel, linux-arm-msm,
Loic Poulain
In-Reply-To: <20170901204118.17123-2-bjorn.andersson@linaro.org>
Hi Bjorn,
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> include/net/bluetooth/bluetooth.h | 2 +-
> net/bluetooth/lib.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
patch has been applied to bluetooth-next tree.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Peter Zijlstra @ 2017-09-01 20:50 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Yonghong Song, rostedt, daniel, netdev, kernel-team
In-Reply-To: <a1905aeb-b49f-d4e8-91ee-a28a92869da1@fb.com>
On Fri, Sep 01, 2017 at 01:29:17PM -0700, Alexei Starovoitov wrote:
> >+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;
> >+}
>
> Peter,
> I believe we're doing it correctly above.
> It's a copy paste of the same logic as in total_time_enabled/running.
> We cannot expose total_time_enabled/running to bpf, since they are
> different counters. The above two are specific to bpf usage.
> See commit log.
No, the patch is atrocious and the usage is wrong.
Exporting a function called 'calc_timer_values' is a horrible violation
of the namespace.
And its wrong because it should be done in conjunction with
perf_event_read_local(). You cannot afterwards call this because you
don't know if the event was active when you read it and you don't have
temporal guarantees; that is, reading these timestamps long after or
before the read is wrong, and this interface allows it.
So no, sorry this is just fail.
^ permalink raw reply
* Re: [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup
From: Marcel Holtmann @ 2017-09-01 20:47 UTC (permalink / raw)
To: Bjorn Andersson
Cc: Gustavo F. Padovan, Johan Hedberg, David S. Miller,
open list:BLUETOOTH DRIVERS, Network Development, LKML,
linux-arm-msm, Loic Poulain, Rob Herring
In-Reply-To: <20170901204118.17123-3-bjorn.andersson@linaro.org>
Hi Bjorn,
> Bluetooth BD address can be retrieved in the same way as
> for wcnss-wlan MAC address. This patch mainly stores the
> local-mac-address property and sets the BD address during
> hci device setup.
>
> Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
> index d00c4fdae924..443bb2099329 100644
> --- a/drivers/bluetooth/btqcomsmd.c
> +++ b/drivers/bluetooth/btqcomsmd.c
> @@ -26,6 +26,7 @@
> struct btqcomsmd {
> struct hci_dev *hdev;
>
> + const bdaddr_t *addr;
> struct rpmsg_endpoint *acl_channel;
> struct rpmsg_endpoint *cmd_channel;
> };
> @@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)
> return 0;
> }
>
> +static int btqcomsmd_setup(struct hci_dev *hdev)
> +{
> + struct btqcomsmd *btq = hci_get_drvdata(hdev);
> + struct sk_buff *skb;
> +
> + skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb))
> + return PTR_ERR(skb);
> + kfree_skb(skb);
> +
> + if (btq->addr) {
> + bdaddr_t bdaddr;
> +
> + /* btq->addr stored with most significant byte first */
> + baswap(&bdaddr, btq->addr);
> + return qca_set_bdaddr_rome(hdev, &bdaddr);
> + }
> +
> + return 0;
> +}
> +
> static int btqcomsmd_probe(struct platform_device *pdev)
> {
> struct btqcomsmd *btq;
> @@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)
> if (IS_ERR(btq->cmd_channel))
> return PTR_ERR(btq->cmd_channel);
>
> + btq->addr = of_get_property(pdev->dev.of_node, "local-mac-address",
> + &ret);
> + if (ret != sizeof(bdaddr_t))
> + btq->addr = NULL;
> +
> hdev = hci_alloc_dev();
> if (!hdev)
> return -ENOMEM;
> @@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)
> hdev->open = btqcomsmd_open;
> hdev->close = btqcomsmd_close;
> hdev->send = btqcomsmd_send;
> + hdev->setup = btqcomsmd_setup;
> hdev->set_bdaddr = qca_set_bdaddr_rome;
I do not like this patch. Why not just set HCI_QUIRK_INVALID_BDADDR and let a userspace tool deal with reading the BD_ADDR from some storage.
Frankly I do not get this WiFI MAC address or BD_ADDR stored in DT. I assumed the DT is suppose to describe hardware and not some value that is normally retrieved for OTP or alike.
Regards
Marcel
^ permalink raw reply
* Re: [iproute PATCH 0/2] Fix and enhance link_gre6
From: Phil Sutter @ 2017-09-01 20:42 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901121333.700624f6@xeon-e3>
Hi Stephen,
On Fri, Sep 01, 2017 at 12:13:33PM -0700, Stephen Hemminger wrote:
> On Fri, 1 Sep 2017 16:08:07 +0200
> Phil Sutter <phil@nwl.cc> wrote:
>
> > Changing a tunnel's flowlabel value was broken if it was set to a
> > non-zero value before. Since the same problem existed for tclass, patch
> > 1 fixes both instances at once.
> >
> > Patch 2 enhances 'ip link show' to also print the tclass value. This
> > change was necessary to properly test the first patch's result.
> >
> > Phil Sutter (2):
> > link_gre6: Fix for changing tclass/flowlabel
> > link_gre6: Print the tunnel's tclass setting
> >
> > ip/link_gre6.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
>
> This doesn't work with net-next where json has been added.
> I fixing it now
Oh, thanks for that. I'm not used to having different states in master
and net-next. :)
Cheers, Phil
^ permalink raw reply
* [PATCH 2/2] Bluetooth: btqcomsmd: BD address setup
From: Bjorn Andersson @ 2017-09-01 20:41 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: David S. Miller, linux-bluetooth, netdev, linux-kernel,
linux-arm-msm, Loic Poulain
In-Reply-To: <20170901204118.17123-1-bjorn.andersson@linaro.org>
From: Loic Poulain <loic.poulain@linaro.org>
Bluetooth BD address can be retrieved in the same way as
for wcnss-wlan MAC address. This patch mainly stores the
local-mac-address property and sets the BD address during
hci device setup.
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/drivers/bluetooth/btqcomsmd.c b/drivers/bluetooth/btqcomsmd.c
index d00c4fdae924..443bb2099329 100644
--- a/drivers/bluetooth/btqcomsmd.c
+++ b/drivers/bluetooth/btqcomsmd.c
@@ -26,6 +26,7 @@
struct btqcomsmd {
struct hci_dev *hdev;
+ const bdaddr_t *addr;
struct rpmsg_endpoint *acl_channel;
struct rpmsg_endpoint *cmd_channel;
};
@@ -100,6 +101,27 @@ static int btqcomsmd_close(struct hci_dev *hdev)
return 0;
}
+static int btqcomsmd_setup(struct hci_dev *hdev)
+{
+ struct btqcomsmd *btq = hci_get_drvdata(hdev);
+ struct sk_buff *skb;
+
+ skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb))
+ return PTR_ERR(skb);
+ kfree_skb(skb);
+
+ if (btq->addr) {
+ bdaddr_t bdaddr;
+
+ /* btq->addr stored with most significant byte first */
+ baswap(&bdaddr, btq->addr);
+ return qca_set_bdaddr_rome(hdev, &bdaddr);
+ }
+
+ return 0;
+}
+
static int btqcomsmd_probe(struct platform_device *pdev)
{
struct btqcomsmd *btq;
@@ -123,6 +145,11 @@ static int btqcomsmd_probe(struct platform_device *pdev)
if (IS_ERR(btq->cmd_channel))
return PTR_ERR(btq->cmd_channel);
+ btq->addr = of_get_property(pdev->dev.of_node, "local-mac-address",
+ &ret);
+ if (ret != sizeof(bdaddr_t))
+ btq->addr = NULL;
+
hdev = hci_alloc_dev();
if (!hdev)
return -ENOMEM;
@@ -135,6 +162,7 @@ static int btqcomsmd_probe(struct platform_device *pdev)
hdev->open = btqcomsmd_open;
hdev->close = btqcomsmd_close;
hdev->send = btqcomsmd_send;
+ hdev->setup = btqcomsmd_setup;
hdev->set_bdaddr = qca_set_bdaddr_rome;
ret = hci_register_dev(hdev);
--
2.12.0
^ permalink raw reply related
* Re: [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Peter Zijlstra @ 2017-09-01 20:41 UTC (permalink / raw)
To: Yonghong Song; +Cc: rostedt, ast, daniel, netdev, kernel-team
In-Reply-To: <20170901165357.465121-2-yhs@fb.com>
On Fri, Sep 01, 2017 at 09:53:54AM -0700, Yonghong Song wrote:
> 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);
>
>
> 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)
Yeah, not going to happen...
Why not do the obvious thing and extend perf_event_read_local() to
optionally return the enabled/running times?
^ permalink raw reply
* [PATCH 1/2] Bluetooth: make baswap src const
From: Bjorn Andersson @ 2017-09-01 20:41 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, David S. Miller
Cc: linux-bluetooth, netdev, linux-kernel, linux-arm-msm,
Loic Poulain
In-Reply-To: <20170901204118.17123-1-bjorn.andersson@linaro.org>
From: Loic Poulain <loic.poulain@linaro.org>
Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/lib.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 01487192f628..020142bb9735 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -233,7 +233,7 @@ static inline void bacpy(bdaddr_t *dst, const bdaddr_t *src)
memcpy(dst, src, sizeof(bdaddr_t));
}
-void baswap(bdaddr_t *dst, bdaddr_t *src);
+void baswap(bdaddr_t *dst, const bdaddr_t *src);
/* Common socket structures and functions */
diff --git a/net/bluetooth/lib.c b/net/bluetooth/lib.c
index aa4cf64e32a6..6048cc07568b 100644
--- a/net/bluetooth/lib.c
+++ b/net/bluetooth/lib.c
@@ -30,10 +30,10 @@
#include <net/bluetooth/bluetooth.h>
-void baswap(bdaddr_t *dst, bdaddr_t *src)
+void baswap(bdaddr_t *dst, const bdaddr_t *src)
{
+ const unsigned char *s = (const unsigned char *)src;
unsigned char *d = (unsigned char *) dst;
- unsigned char *s = (unsigned char *) src;
unsigned int i;
for (i = 0; i < 6; i++)
--
2.12.0
^ permalink raw reply related
* [PATCH 0/2] btqcomsmd: Allow specifying board mac address
From: Bjorn Andersson @ 2017-09-01 20:41 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo Padovan, Johan Hedberg
Cc: David S. Miller, linux-bluetooth, netdev, linux-kernel,
linux-arm-msm, Loic Poulain
The btqcomsmd hardware lacks persistent storage of its mac address, so this
needs to be configured during initialization. The second patch in this series
reads the mac address from DT and does this, allowing the boot loader to
populate this board specific information.
Loic Poulain (2):
Bluetooth: make baswap src const
Bluetooth: btqcomsmd: BD address setup
drivers/bluetooth/btqcomsmd.c | 28 ++++++++++++++++++++++++++++
include/net/bluetooth/bluetooth.h | 2 +-
net/bluetooth/lib.c | 4 ++--
3 files changed, 31 insertions(+), 3 deletions(-)
--
2.12.0
^ permalink raw reply
* Re: [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Alexei Starovoitov @ 2017-09-01 20:29 UTC (permalink / raw)
To: Yonghong Song, peterz, rostedt, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-2-yhs@fb.com>
On 9/1/17 9:53 AM, Yonghong Song wrote:
> 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>
...
> +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;
> +}
Peter,
I believe we're doing it correctly above.
It's a copy paste of the same logic as in total_time_enabled/running.
We cannot expose total_time_enabled/running to bpf, since they are
different counters. The above two are specific to bpf usage.
See commit log.
for the whole set:
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH 13/31] timer: Remove meaningless .data/.function assignments
From: Jens Axboe @ 2017-09-01 20:07 UTC (permalink / raw)
To: Kees Cook, Thomas Gleixner
Cc: Krzysztof Halasa, Aditya Shankar, Ganesh Krishna,
Greg Kroah-Hartman, netdev, linux-wireless, devel, linux-kernel
In-Reply-To: <1504222183-61202-14-git-send-email-keescook@chromium.org>
On 08/31/2017 05:29 PM, Kees Cook wrote:
> Several timer users needlessly reset their .function/.data fields during
> their timer callback, but nothing else changes them. Some users do not
> use their .data field at all. Each instance is removed here.
For amiflop:
Acked-by: Jens Axboe <axboe@kernel.dk>
--
Jens Axboe
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Andrew Lunn @ 2017-09-01 19:44 UTC (permalink / raw)
To: Florian Fainelli; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <5a3fa91f-fb1f-e5ea-987d-973dee3bf8eb@gmail.com>
> I see what you mean, so something along the lines of just:
>
> tc bind dev swp0p0 queue 0 master queue 16
>
> without having to specify the master network device since it's implicit,
> I kind of like that.
Yes, that is better.
Andrew
^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Kees Cook @ 2017-09-01 19:40 UTC (permalink / raw)
To: Mike Galbraith
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jKiGFAp8Lp2Nt1yHg_nYsY_rfrUjMgOPTNxMLkbQ8V-ng@mail.gmail.com>
On Fri, Sep 1, 2017 at 11:58 AM, Kees Cook <keescook@chromium.org> wrote:
> On Fri, Sep 1, 2017 at 10:52 AM, Mike Galbraith <efault@gmx.de> wrote:
>> On Fri, 2017-09-01 at 10:12 -0700, Kees Cook wrote:
>>> On Fri, Sep 1, 2017 at 6:09 AM, Mike Galbraith <efault@gmx.de> wrote:
>>> > On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
>>> >> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
>>> >> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
>>> >> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
>>> >> > >>
>>> >> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
>>> >> > >>
>>> >> > >> 4.8.5: WARN, eventual kernel hang
>>> >> > >> 6.3.1, 7.0.1: WARN, but continues working
>>> >> > >
>>> >> > > Yeah, that's correct. I find that troubling, simply because this gcc
>>> >> > > version has been through one hell of a lot of kernels with me. Yeah, I
>>> >> > > know, that doesn't exempt it from having bugs, but color me suspicious.
>>> >> >
>>> >> > I still can't hit this with a 4.8.5 build. :(
>>> >> >
>>> >> > With _RATELIMIT removed, this should, in theory, report whatever goes
>>> >> > negative first...
>>> >>
>>> >> I applied the other patch you posted, and built with gcc-6.3.1 to
>>> >> remove the gcc-4.8.5 aspect. Look below the resulting splat.
>>> >
>>> > Grr, that one has a in6_dev_getx() line missing for the first
>>> > increment, where things go pear shaped.
>>> >
>>> > With that added, looking at counter both before, and after incl, with a
>>> > trace_printk() in the exception handler showing it doing its saturate
>>> > thing, irqs disabled across the whole damn refcount_inc(), and even
>>> > booting box nr_cpus=1 for extra credit...
>>> >
>>> > HTH can that first refcount_inc() get there?
>>> >
>>> > # tracer: nop
>>> > #
>>> > # _-----=> irqs-off
>>> > # / _----=> need-resched
>>> > # | / _---=> hardirq/softirq
>>> > # || / _--=> preempt-depth
>>> > # ||| / delay
>>> > # TASK-PID CPU# |||| TIMESTAMP FUNCTION
>>> > # | | | |||| | |
>>> > systemd-1 [000] d..1 1.937284: in6_dev_getx: PRE refs.counter:3
>>> > systemd-1 [000] d..1 1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
>>> > systemd-1 [000] d..1 1.937296: in6_dev_getx: POST refs.counter:-1073741824
>>>
>>> O_o
>>>
>>> Can you paste the disassembly of in6_dev_getx? I can't understand how
>>> we're landing in the exception handler.
>>
>> I was hoping you'd say that.
>>
>> 0xffffffff816b2f72 <+0>: push %rbp
>> 0xffffffff816b2f73 <+1>: mov %rsp,%rbp
>> 0xffffffff816b2f76 <+4>: push %r12
>> 0xffffffff816b2f78 <+6>: push %rbx
>> 0xffffffff816b2f79 <+7>: incl %gs:0x7e95a2d0(%rip) # 0xd250 <__preempt_count>
>> 0xffffffff816b2f80 <+14>: mov 0x308(%rdi),%rbx
>> 0xffffffff816b2f87 <+21>: test %rbx,%rbx
>> 0xffffffff816b2f8a <+24>: je 0xffffffff816b2feb <in6_dev_getx+121>
>> 0xffffffff816b2f8c <+26>: callq *0xffffffff81c35a00
>> 0xffffffff816b2f93 <+33>: mov %rax,%r12
>> 0xffffffff816b2f96 <+36>: callq *0xffffffff81c35a10
>> 0xffffffff816b2f9d <+43>: mov 0x769ad4(%rip),%rsi # 0xffffffff81e1ca78 <trace_printk_fmt.21733>
>> 0xffffffff816b2fa4 <+50>: mov 0xf0(%rbx),%edx
>> 0xffffffff816b2faa <+56>: mov $0xffffffff816b2f8c,%rdi
>> 0xffffffff816b2fb1 <+63>: callq 0xffffffff81171fc0 <__trace_bprintk>
>> 0xffffffff816b2fb6 <+68>: lock incl 0xf0(%rbx)
>> 0xffffffff816b2fbd <+75>: js 0xffffffff816b2fbf <in6_dev_getx+77>
>> 0xffffffff816b2fbf <+77>: lea 0xf0(%rbx),%rcx
>> 0xffffffff816b2fc6 <+84>: (bad)
>> 0xffffffff816b2fc8 <+86>: mov 0x769a99(%rip),%rsi # 0xffffffff81e1ca68 <trace_printk_fmt.21744>
>> 0xffffffff816b2fcf <+93>: mov 0xf0(%rbx),%edx
>> 0xffffffff816b2fd5 <+99>: mov $0xffffffff816b2f8c,%rdi
>> 0xffffffff816b2fdc <+106>: callq 0xffffffff81171fc0 <__trace_bprintk>
>> 0xffffffff816b2fe1 <+111>: mov %r12,%rdi
>> 0xffffffff816b2fe4 <+114>: callq *0xffffffff81c35a08
>> 0xffffffff816b2feb <+121>: decl %gs:0x7e95a25e(%rip) # 0xd250 <__preempt_count>
>> 0xffffffff816b2ff2 <+128>: mov %rbx,%rax
>> 0xffffffff816b2ff5 <+131>: pop %rbx
>> 0xffffffff816b2ff6 <+132>: pop %r12
>> 0xffffffff816b2ff8 <+134>: pop %rbp
>> 0xffffffff816b2ff9 <+135>: retq
>>
>> I don't get the section business at all, +75 looks to me like we're
>> gonna trap no matter what.. as we appear to be doing.
>
> The section stuff is supposed to be a trick to push the error case off
> into the .text.unlikely area to avoid needing a jmp over the handler
> and with possibly some redundancy removal done by the compiler (though
> this appears to be rather limited) if it notices a bunch of error
> paths are the same. However, in your disassembly, it's inline (!!) in
> the code, as if "pushsection" and "popsection" were entirely ignored.
>
> And when I make my own in6_dev_getx(), I see the same disassembly:
>
> 0xffffffff818a757b <+181>: lock incl 0x1e0(%rbx)
> 0xffffffff818a7582 <+188>: js 0xffffffff818a7584 <in6_dev_getx+190>
> 0xffffffff818a7584 <+190>: lea 0x1e0(%rbx),%rcx
> 0xffffffff818a758b <+197>: (bad)
>
> Which is VERY different from how it looks in other places!
Found it.
If the compiler already pushed the entire function into
.text.unlikely, x86-refcount's .pushsection doesn't do any good
(obviously). Durrr.
.section .text.unlikely,"ax",@progbits
.type in6_dev_getx, @function
in6_dev_getx:
.LFB4673:
.loc 2 4128 0
.cfi_startproc
...
lock; incl 480(%rbx)
js 111f
.pushsection .text.unlikely
111: lea 480(%rbx), %rcx
112: .byte 0x0f, 0xff
.popsection
113:
I will get this fixed. Thank you again for helping track this down!
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN
From: Eric W. Biederman @ 2017-09-01 19:29 UTC (permalink / raw)
To: Prakash Sangappa; +Cc: David Miller, linux-kernel, netdev, drepper
In-Reply-To: <ca086e10-764c-4389-808e-43fd1ace4e0c@oracle.com>
Prakash Sangappa <prakash.sangappa@oracle.com> writes:
> On 8/30/17 10:41 AM, ebiederm@xmission.com wrote:
>> Prakash Sangappa <prakash.sangappa@oracle.com> writes:
>>
>>
>>> With regards to security, the question basically is what is the consequence
>>> of passing the wrong id. As I understand it, Interpreting the id to be pid
>>> or tid, the effective uid and gid will be the same. It would be a problem
>>> only if the incorrect interpretation of the id would refer a different process.
>>> But that cannot happen as the the global tid(gettid() of a thread is
>>> unique.
>> There is also the issue that the receiving process could look, not see
>> the pid in proc and assume the sending process is dead. That I suspect
>> is the larger danger.
>>
>
> Will this not be a bug in the application, if it is sending the wrong
> id?
No. It could be deliberate and malicious.
>>> As long as the thread is alive, that id cannot reference another process / thread.
>>> Unless the thread were to exit and the id gets recycled and got used for another
>>> thread or process. This would be no different from a process exiting and its
>>> pid getting recycled which is the case now.
>> Largely I agree.
>>
>> If all you want are pid translations I suspect the are far easier ways
>> thant updating the SCM_CREDENTIALS code.
>
> What would be an another easier & efficient way of doing pid translation?
>
> Should a new API/mechanism be considered mainly for pid translation purpose
> for use with pid namespaces, say based on 'pipe' something similar to
> I_SENDFD?
There are proc files that provide all of the pids of a process you can
read those.
Other possibilities exist if you want to go that fast.
Eric
^ permalink raw reply
* Re: [net-next PATCH] bpf: sockmap update/simplify memory accounting scheme
From: Alexei Starovoitov @ 2017-09-01 19:25 UTC (permalink / raw)
To: John Fastabend, davem; +Cc: netdev, daniel
In-Reply-To: <20170901182926.8981.77450.stgit@john-Precision-Tower-5810>
On 9/1/17 11:29 AM, John Fastabend wrote:
> Instead of tracking wmem_queued and sk_mem_charge by incrementing
> in the verdict SK_REDIRECT paths and decrementing in the tx work
> path use skb_set_owner_w and sock_writeable helpers. This solves
> a few issues with the current code. First, in SK_REDIRECT inc on
> sk_wmem_queued and sk_mem_charge were being done without the peers
> sock lock being held. Under stress this can result in accounting
> errors when tx work and/or multiple verdict decisions are working
> on the peer psock.
>
> Additionally, this cleans up the code because we can rely on the
> default destructor to decrement memory accounting on kfree_skb. Also
> this will trigger sk_write_space when space becomes available on
> kfree_skb() which wasn't happening before and prevent __sk_free
> from being called until all in-flight packets are completed.
>
> Fixes: 174a79ff9515 ("bpf: sockmap with sk redirect support")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
> Acked-by: Daniel Borkmann <daniel@iogearbox.net>
thanks. it's cleaner indeed.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Mike Galbraith @ 2017-09-01 19:24 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
Reshetova, Elena, Network Development
In-Reply-To: <CAGXu5jKiGFAp8Lp2Nt1yHg_nYsY_rfrUjMgOPTNxMLkbQ8V-ng@mail.gmail.com>
On Fri, 2017-09-01 at 11:58 -0700, Kees Cook wrote:
>
> The section stuff is supposed to be a trick to push the error case off
> into the .text.unlikely area to avoid needing a jmp over the handler
> and with possibly some redundancy removal done by the compiler (though
> this appears to be rather limited) if it notices a bunch of error
> paths are the same. However, in your disassembly, it's inline (!!) in
> the code, as if "pushsection" and "popsection" were entirely ignored.
>
> And when I make my own in6_dev_getx(), I see the same disassembly:
>
> 0xffffffff818a757b <+181>: lock incl 0x1e0(%rbx)
> 0xffffffff818a7582 <+188>: js 0xffffffff818a7584 <in6_dev_getx+190>
> 0xffffffff818a7584 <+190>: lea 0x1e0(%rbx),%rcx
> 0xffffffff818a758b <+197>: (bad)
>
> Which is VERY different from how it looks in other places!
>
> e.g. from lkdtm_REFCOUNT_INC_SATURATED:
>
> 0xffffffff815657df <+47>: lock incl -0xc(%rbp)
> 0xffffffff815657e3 <+51>: js 0xffffffff81565cac
> ...
> 0xffffffff81565cac: lea -0xc(%rbp),%rcx
> 0xffffffff81565cb0: (bad)
>
> So, at least I can reproduce this in the build now. I must not be
> exercising these paths. FWIW, this is with Ubuntu's 6.3.0 gcc.
>
> I'll try to figure out what's going on here...
Heh, make in6_dev_getx() __always_inline.
swapper/0-1 [000] d..1 1.438587: ip6_route_init_special_entries: PRE refs.counter:3
swapper/0-1 [000] d..1 1.438590: ip6_route_init_special_entries: POST refs.counter:4
swapper/0-1 [000] d..1 1.438591: ip6_route_init_special_entries: PRE refs.counter:4
swapper/0-1 [000] d..1 1.438592: ip6_route_init_special_entries: POST refs.counter:5
swapper/0-1 [000] d..1 1.438592: ip6_route_init_special_entries: PRE refs.counter:5
swapper/0-1 [000] d..1 1.438593: ip6_route_init_special_entries: POST refs.counter:6
-Mike
^ permalink raw reply
* Re: [RFC net-next 0/8] net: dsa: Multi-queue awareness
From: Florian Fainelli @ 2017-09-01 19:21 UTC (permalink / raw)
To: Andrew Lunn; +Cc: jiri, jhs, netdev, davem, xiyou.wangcong, vivien.didelot
In-Reply-To: <20170901185012.GA8122@lunn.ch>
On 09/01/2017 11:50 AM, Andrew Lunn wrote:
> On Fri, Sep 01, 2017 at 11:27:43AM -0700, Florian Fainelli wrote:
>> On 09/01/2017 10:55 AM, Andrew Lunn wrote:
>>> Hi Florian
>>>
>>>>>> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>>>
>>> It this the eth0 i don't like here. Why not in the implementation just
>>> use something like netdev_master_upper_dev_get('sw0p0')? Or does
>>
>> Last I brought this up with Jiri that we should link DSA network devices
>> to their master network deviecs with netdev_upper_dev_link() he said
>> this was not appropriate for DSA slave network devices, but I can't
>> remember why, I would assume that any stacked device set up would do that.
>
> There is some form a linking going, our device names show that:
>
> 9: lan5@eth1: <BROADCAST,MULTICAST> mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default qlen 1000
> link/ether da:87:2a:03:cf:16 brd ff:ff:ff:ff:ff:ff
This is because iproute2 is linking the devices based on what
ndo_get_iflink() returns.
>
>> In any case, we need to establish a mapping so we have to specify at
>> least the target device's queue number. It is quite similar in premise
>> to e.g: enslaving a network device to a bridge port:
>>
>> ip link set dev eth0 master br0
>
> But here br0 is absolutely required, we have to say which bridge the
> slave port should be a member of.
Right,
>
> But what good is eth0 in
>
> tc bind dev sw0p0 queue 0 dev eth0 queue 16
>
> As i said suggesting, you have to somehow verify that eth0 is the
> conduit interface sw0p0 is using. Which makes the parameter pointless.
> Determine it from the sw0p0 somehow.
I see what you mean, so something along the lines of just:
tc bind dev swp0p0 queue 0 master queue 16
without having to specify the master network device since it's implicit,
I kind of like that.
--
Florian
^ permalink raw reply
* RE: [PATCH 0/2] i40e: fix firmware update
From: Keller, Jacob E @ 2017-09-01 19:19 UTC (permalink / raw)
To: Stefan Assmann, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, davem@davemloft.net, Kirsher, Jeffrey T
In-Reply-To: <20170901140234.7840-1-sassmann@kpanic.de>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Stefan Assmann
> Sent: Friday, September 01, 2017 7:03 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; davem@davemloft.net; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; sassmann@kpanic.de
> Subject: [PATCH 0/2] i40e: fix firmware update
>
> The first patch fixes the firmware update which is currently broken and
> results in a bad flash (corrupt firmware). Recovery is possible with a
> fixed driver.
> The second patch reverts a commit that causes the firmware checksum
> verification to fail right after a successful flash. This is related to
> a recent workqueue change. Haven't gotten to the bottom of this yet, but
> for the sake of a smooth firmware update experience let's revert the
> commit for now.
Hi Stefan,
Thanks for these patches, I apologize for the time it took for us to respond to this.
The first patch is functionally correct, and I'm surprised we missed sending an equivalent ourselves. It looks like some related changes occurred around this code, and we failed to submit the patch.
I think Jeff would prefer if we send the version based directly on the out-of-tree code, which I will be reviving and submitting shortly.
The second issue I believe is not fixed correctly by the patch, I'm unsure why exactly changing the WQ would cause this but I believe that a similar patch which creates a non-locked version of i40e_nvm_read_buffer() will resolve this, and I will be sending that patch as well, which I believe is the real fix versus halting the work queue.
Thanks,
Jake
>
> Stefan Assmann (2):
> i40e: use non-locking i40e_read_nvm_word() function during nvmupdate
> Revert "i40e: remove WQ_UNBOUND and the task limit of our workqueue"
>
> drivers/net/ethernet/intel/i40e/i40e_main.c | 12 +++++-------
> drivers/net/ethernet/intel/i40e/i40e_nvm.c | 24 ++++++++++++++++++++++--
> 2 files changed, 27 insertions(+), 9 deletions(-)
>
> --
> 2.13.5
^ permalink raw reply
* Re: [PATCH 1/1] bpf: take advantage of stack_depth tracking in powerpc JIT
From: Daniel Borkmann @ 2017-09-01 19:15 UTC (permalink / raw)
To: Sandipan Das, mpe; +Cc: naveen.n.rao, ast, netdev, linuxppc-dev
In-Reply-To: <20170901185301.17978-1-sandipan@linux.vnet.ibm.com>
On 09/01/2017 08:53 PM, Sandipan Das wrote:
> Take advantage of stack_depth tracking, originally introduced for
> x64, in powerpc JIT as well. Round up allocated stack by 16 bytes
> to make sure it stays aligned for functions called from JITed bpf
> program.
>
> Signed-off-by: Sandipan Das <sandipan@linux.vnet.ibm.com>
Awesome, thanks for following up! :)
^ 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