* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Nikolay Aleksandrov @ 2017-05-19 15:41 UTC (permalink / raw)
To: Xin Long, network dev
Cc: davem, cera, Satish Ashok, Stephen Hemminger, bridge
In-Reply-To: <6e78bfea3969ca1a937b37e28e796a3047b82615.1495203629.git.lucien.xin@gmail.com>
On 5/19/17 5:20 PM, Xin Long wrote:
> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
> kernel hello and hold timers"), bridge would not start hello_timer if
> stp_enabled is not KERNEL_STP when br_dev_open.
>
> The problem is even if users set stp_enabled with KERNEL_STP later,
> the timer will still not be started. It causes that KERNEL_STP can
> not really work. Users have to re-ifup the bridge to avoid this.
>
> This patch is to fix it by starting br->hello_timer when enabling
> KERNEL_STP in br_stp_start.
>
> As an improvement, it's also to start hello_timer again only when
> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
> no reason to start the timer again when it's NO_STP.
>
> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and hold timers")
> Reported-by: Haidong Li <haili@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_stp_if.c | 1 +
> net/bridge/br_stp_timer.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
> index 08341d2..0db8102 100644
> --- a/net/bridge/br_stp_if.c
> +++ b/net/bridge/br_stp_if.c
> @@ -179,6 +179,7 @@ static void br_stp_start(struct net_bridge *br)
> br_debug(br, "using kernel STP\n");
>
> /* To start timers on any ports left in blocking */
> + mod_timer(&br->hello_timer, jiffies + br->hello_time);
> br_port_state_selection(br);
> }
>
> diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
> index c98b3e5..60b6fe2 100644
> --- a/net/bridge/br_stp_timer.c
> +++ b/net/bridge/br_stp_timer.c
> @@ -40,7 +40,7 @@ static void br_hello_timer_expired(unsigned long arg)
> if (br->dev->flags & IFF_UP) {
> br_config_bpdu_generation(br);
>
> - if (br->stp_enabled != BR_USER_STP)
> + if (br->stp_enabled == BR_KERNEL_STP)
> mod_timer(&br->hello_timer,
> round_jiffies(jiffies + br->hello_time));
> }
>
+CC Bridge maintainers & fixes commit author
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* [PATCH net] sctp: fix ICMP processing if skb is non-linear
From: Davide Caratti @ 2017-05-19 15:34 UTC (permalink / raw)
To: netdev, linux-sctp; +Cc: David S. Miller, Xin Long
when the ICMP packet is carried by a paged skb, sctp_err_lookup() may fail
validation even if the payload contents match an open socket: as a
consequence, sometimes ICMPs are wrongly ignored. Use skb_header_pointer()
to retrieve encapsulated SCTP headers, to ensure that ICMP payloads are
validated correctly, also when skbs are non-linear.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
include/net/sctp/sctp.h | 2 +-
net/sctp/input.c | 29 +++++++++++++++++++----------
net/sctp/ipv6.c | 2 +-
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/include/net/sctp/sctp.h b/include/net/sctp/sctp.h
index 069582e..1b8c16b 100644
--- a/include/net/sctp/sctp.h
+++ b/include/net/sctp/sctp.h
@@ -152,7 +152,7 @@ void sctp_v4_err(struct sk_buff *skb, u32 info);
void sctp_hash_endpoint(struct sctp_endpoint *);
void sctp_unhash_endpoint(struct sctp_endpoint *);
struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *,
- struct sctphdr *, struct sctp_association **,
+ struct sctp_association **,
struct sctp_transport **);
void sctp_err_finish(struct sock *, struct sctp_transport *);
void sctp_icmp_frag_needed(struct sock *, struct sctp_association *,
diff --git a/net/sctp/input.c b/net/sctp/input.c
index 0e06a27..7f3f983 100644
--- a/net/sctp/input.c
+++ b/net/sctp/input.c
@@ -469,19 +469,19 @@ void sctp_icmp_proto_unreachable(struct sock *sk,
/* Common lookup code for icmp/icmpv6 error handler. */
struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
- struct sctphdr *sctphdr,
struct sctp_association **app,
struct sctp_transport **tpp)
{
+ struct sctp_init_chunk _chunkhdr, *chunkhdr;
+ struct sctphdr _sctphdr, *sctphdr;
union sctp_addr saddr;
union sctp_addr daddr;
struct sctp_af *af;
struct sock *sk = NULL;
struct sctp_association *asoc;
struct sctp_transport *transport = NULL;
- struct sctp_init_chunk *chunkhdr;
- __u32 vtag = ntohl(sctphdr->vtag);
- int len = skb->len - ((void *)sctphdr - (void *)skb->data);
+ int offset;
+ __u32 vtag;
*app = NULL; *tpp = NULL;
@@ -515,14 +515,23 @@ struct sock *sctp_err_lookup(struct net *net, int family, struct sk_buff *skb,
* or the chunk type or the Initiate Tag does not match, silently
* discard the packet.
*/
+ offset = skb_transport_offset(skb);
+ sctphdr = skb_header_pointer(skb, offset, sizeof(_sctphdr), &_sctphdr);
+ if (unlikely(!sctphdr))
+ goto out;
+
+ vtag = ntohl(sctphdr->vtag);
if (vtag == 0) {
- chunkhdr = (void *)sctphdr + sizeof(struct sctphdr);
- if (len < sizeof(struct sctphdr) + sizeof(sctp_chunkhdr_t)
- + sizeof(__be32) ||
+ offset += sizeof(_sctphdr);
+ /* chunk header + first 4 octects of init header */
+ chunkhdr = skb_header_pointer(skb, offset,
+ sizeof(struct sctp_chunkhdr) +
+ sizeof(__be32), &_chunkhdr);
+ if (!chunkhdr ||
chunkhdr->chunk_hdr.type != SCTP_CID_INIT ||
- ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag) {
+ ntohl(chunkhdr->init_hdr.init_tag) != asoc->c.my_vtag)
goto out;
- }
+
} else if (vtag != asoc->c.peer_vtag) {
goto out;
}
@@ -585,7 +594,7 @@ void sctp_v4_err(struct sk_buff *skb, __u32 info)
savesctp = skb->transport_header;
skb_reset_network_header(skb);
skb_set_transport_header(skb, ihlen);
- sk = sctp_err_lookup(net, AF_INET, skb, sctp_hdr(skb), &asoc, &transport);
+ sk = sctp_err_lookup(net, AF_INET, skb, &asoc, &transport);
/* Put back, the original values. */
skb->network_header = saveip;
skb->transport_header = savesctp;
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index 142b70e..d72c8d5 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -157,7 +157,7 @@ static void sctp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
savesctp = skb->transport_header;
skb_reset_network_header(skb);
skb_set_transport_header(skb, offset);
- sk = sctp_err_lookup(net, AF_INET6, skb, sctp_hdr(skb), &asoc, &transport);
+ sk = sctp_err_lookup(net, AF_INET6, skb, &asoc, &transport);
/* Put back, the original pointers. */
skb->network_header = saveip;
skb->transport_header = savesctp;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v5 net-next 5/7] net: fix documentation of struct scm_timestamping
From: Willem de Bruijn @ 2017-05-19 15:23 UTC (permalink / raw)
To: Miroslav Lichvar; +Cc: Network Development, Richard Cochran, Willem de Bruijn
In-Reply-To: <20170519101118.GD21003@localhost>
On Fri, May 19, 2017 at 6:11 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> On Thu, May 18, 2017 at 03:38:30PM -0400, Willem de Bruijn wrote:
>> On Thu, May 18, 2017 at 10:07 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>> > +Note that if the SO_TIMESTAMP or SO_TIMESTAMPNS option is enabled
>> > +together with SO_TIMESTAMPING using SOF_TIMESTAMPING_SOFTWARE, a false
>> > +software timestamp will be generated in the recvmsg() call and passed
>> > +in ts[0] when a real software timestamp is missing.
>>
>> With receive software timestamping this is expected behavior? I would make
>> explicit that this happens even on tx timestamps.
>
> How about adding ", e.g. when receive timestamping is enabled
> between receiving the message and the recvmsg() call, or it is a
> message with a hardware transmit timestamp." ?
Perhaps even more brief "This happens also on hardware tx timestamps."
>> > For this reason it
>> > +is not recommended to combine SO_TIMESTAMP(NS) with SO_TIMESTAMPING.
>>
>> And I'd remove this. The extra timestamp is harmless, and we may be missing
>> other reasons why someone would want to enable both on the same socket.
>
> Ok. I'm just concerned people will inadvertently use the timestamp as
> a real timestamp and then wonder why SW TX timestamping is so bad. I
> have fallen into this trap.
So have I. It is equally surprising when only enabling SO_TIMESTAMP and
observing out of order timestamps. It is certainly worth calling out.
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Ivan Vecera @ 2017-05-19 15:17 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Xin Long, network dev, David Miller
In-Reply-To: <3c987d40-6e47-3d7c-c6b9-b46ee5afcd57@cumulusnetworks.com>
2017-05-19 17:05 GMT+02:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 5/19/17 6:03 PM, Ivan Vecera wrote:
>>
>> 2017-05-19 16:57 GMT+02:00 Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com>:
>>>
>>> On 5/19/17 5:51 PM, Ivan Vecera wrote:
>>>>
>>>>
>>>> 2017-05-19 16:45 GMT+02:00 Nikolay Aleksandrov
>>>> <nikolay@cumulusnetworks.com>:
>>>>>
>>>>>
>>>>> On 5/19/17 5:20 PM, Xin Long wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>>>>> kernel hello and hold timers"), bridge would not start hello_timer if
>>>>>> stp_enabled is not KERNEL_STP when br_dev_open.
>>>>>>
>>>>>> The problem is even if users set stp_enabled with KERNEL_STP later,
>>>>>> the timer will still not be started. It causes that KERNEL_STP can
>>>>>> not really work. Users have to re-ifup the bridge to avoid this.
>>>>>>
>>>>>> This patch is to fix it by starting br->hello_timer when enabling
>>>>>> KERNEL_STP in br_stp_start.
>>>>>>
>>>>>> As an improvement, it's also to start hello_timer again only when
>>>>>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>>>>>> no reason to start the timer again when it's NO_STP.
>>>>>>
>>>>>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>>>>> kernel
>>>>>> hello and hold timers")
>>>>>> Reported-by: Haidong Li <haili@redhat.com>
>>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>>> ---
>>>>>> net/bridge/br_stp_if.c | 1 +
>>>>>> net/bridge/br_stp_timer.c | 2 +-
>>>>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>
>>>>> This doesn't make much sense to me, how do you change from USER_STP to
>>>>> KERNEL_STP without first going through NO_STP ?
>>>>>
>>>>> If you go through NO_STP then all will be fine because br_stp_stop will
>>>>> restart
>>>>> the timers if the previous val was USER_STP.
>>>>>
>>>> The problem occurs when KERNEL_STP is enabled if the bridge itself is
>>>> already
>>>> up. Then the hello_timer is not started. If the hello and hold timers
>>>> should run only
>>>> when KERNEL_STP is used then there are another problematic places
>>>> (will send follow-up).
>>>>
>>>> Ivan
>>>>
>>>
>>> Oh, the problem seems to be rather going from NO_STP -> KERNEL_STP only
>>> then, because you cannot do direct USER_STP -> KERNEL_STP.
>>>
>> No only NO_STP->KERNEL_STP but KERNEL_STP->NO_STP as well as
>> USER_STP->NO_STP:
>>
>> 1) NO_STP->KERNEL_STP issue
>> hello_timer should be started in br_stp_start() - this patch
>>
>
> Right, I was talking only about this patch. By the way what about
> the port hold_timers ? This patch only starts the hello_timer.
The hold_timers should be started indirectly from br_transmit_config()
called from
br_config_bpdu_generation() called from hello_timer_expired() handler.
I.
>> 2) KERNEL_STP->NO_STP issue
>> hello timer and hold timers should be stopped (deleted) in br_stp_stop()
>>
>> 3) USER_STP->NO_STP issue
>> hello timer and hold timers should NOT be started in br_stp_stop()
>>
>
> Yep, ack.
>
>> Ivan
>>
>
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Nikolay Aleksandrov @ 2017-05-19 15:05 UTC (permalink / raw)
To: ivan.vecera; +Cc: Xin Long, network dev, David Miller
In-Reply-To: <CA+Nub0Zi6-2PL3+6v5r3t3CD6NowJY_j=bd+x4NA42KKiPPp7A@mail.gmail.com>
On 5/19/17 6:03 PM, Ivan Vecera wrote:
> 2017-05-19 16:57 GMT+02:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
>> On 5/19/17 5:51 PM, Ivan Vecera wrote:
>>>
>>> 2017-05-19 16:45 GMT+02:00 Nikolay Aleksandrov
>>> <nikolay@cumulusnetworks.com>:
>>>>
>>>> On 5/19/17 5:20 PM, Xin Long wrote:
>>>>>
>>>>>
>>>>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>>>> kernel hello and hold timers"), bridge would not start hello_timer if
>>>>> stp_enabled is not KERNEL_STP when br_dev_open.
>>>>>
>>>>> The problem is even if users set stp_enabled with KERNEL_STP later,
>>>>> the timer will still not be started. It causes that KERNEL_STP can
>>>>> not really work. Users have to re-ifup the bridge to avoid this.
>>>>>
>>>>> This patch is to fix it by starting br->hello_timer when enabling
>>>>> KERNEL_STP in br_stp_start.
>>>>>
>>>>> As an improvement, it's also to start hello_timer again only when
>>>>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>>>>> no reason to start the timer again when it's NO_STP.
>>>>>
>>>>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel
>>>>> hello and hold timers")
>>>>> Reported-by: Haidong Li <haili@redhat.com>
>>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>>> ---
>>>>> net/bridge/br_stp_if.c | 1 +
>>>>> net/bridge/br_stp_timer.c | 2 +-
>>>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>
>>>> This doesn't make much sense to me, how do you change from USER_STP to
>>>> KERNEL_STP without first going through NO_STP ?
>>>>
>>>> If you go through NO_STP then all will be fine because br_stp_stop will
>>>> restart
>>>> the timers if the previous val was USER_STP.
>>>>
>>> The problem occurs when KERNEL_STP is enabled if the bridge itself is
>>> already
>>> up. Then the hello_timer is not started. If the hello and hold timers
>>> should run only
>>> when KERNEL_STP is used then there are another problematic places
>>> (will send follow-up).
>>>
>>> Ivan
>>>
>>
>> Oh, the problem seems to be rather going from NO_STP -> KERNEL_STP only
>> then, because you cannot do direct USER_STP -> KERNEL_STP.
>>
> No only NO_STP->KERNEL_STP but KERNEL_STP->NO_STP as well as USER_STP->NO_STP:
>
> 1) NO_STP->KERNEL_STP issue
> hello_timer should be started in br_stp_start() - this patch
>
Right, I was talking only about this patch. By the way what about
the port hold_timers ? This patch only starts the hello_timer.
> 2) KERNEL_STP->NO_STP issue
> hello timer and hold timers should be stopped (deleted) in br_stp_stop()
>
> 3) USER_STP->NO_STP issue
> hello timer and hold timers should NOT be started in br_stp_stop()
>
Yep, ack.
> Ivan
>
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Ivan Vecera @ 2017-05-19 15:03 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Xin Long, network dev, David Miller
In-Reply-To: <026b6024-4ec8-5060-f19a-163ed485207c@cumulusnetworks.com>
2017-05-19 16:57 GMT+02:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 5/19/17 5:51 PM, Ivan Vecera wrote:
>>
>> 2017-05-19 16:45 GMT+02:00 Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com>:
>>>
>>> On 5/19/17 5:20 PM, Xin Long wrote:
>>>>
>>>>
>>>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>>> kernel hello and hold timers"), bridge would not start hello_timer if
>>>> stp_enabled is not KERNEL_STP when br_dev_open.
>>>>
>>>> The problem is even if users set stp_enabled with KERNEL_STP later,
>>>> the timer will still not be started. It causes that KERNEL_STP can
>>>> not really work. Users have to re-ifup the bridge to avoid this.
>>>>
>>>> This patch is to fix it by starting br->hello_timer when enabling
>>>> KERNEL_STP in br_stp_start.
>>>>
>>>> As an improvement, it's also to start hello_timer again only when
>>>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>>>> no reason to start the timer again when it's NO_STP.
>>>>
>>>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel
>>>> hello and hold timers")
>>>> Reported-by: Haidong Li <haili@redhat.com>
>>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>>> ---
>>>> net/bridge/br_stp_if.c | 1 +
>>>> net/bridge/br_stp_timer.c | 2 +-
>>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>
>>> This doesn't make much sense to me, how do you change from USER_STP to
>>> KERNEL_STP without first going through NO_STP ?
>>>
>>> If you go through NO_STP then all will be fine because br_stp_stop will
>>> restart
>>> the timers if the previous val was USER_STP.
>>>
>> The problem occurs when KERNEL_STP is enabled if the bridge itself is
>> already
>> up. Then the hello_timer is not started. If the hello and hold timers
>> should run only
>> when KERNEL_STP is used then there are another problematic places
>> (will send follow-up).
>>
>> Ivan
>>
>
> Oh, the problem seems to be rather going from NO_STP -> KERNEL_STP only
> then, because you cannot do direct USER_STP -> KERNEL_STP.
>
No only NO_STP->KERNEL_STP but KERNEL_STP->NO_STP as well as USER_STP->NO_STP:
1) NO_STP->KERNEL_STP issue
hello_timer should be started in br_stp_start() - this patch
2) KERNEL_STP->NO_STP issue
hello timer and hold timers should be stopped (deleted) in br_stp_stop()
3) USER_STP->NO_STP issue
hello timer and hold timers should NOT be started in br_stp_stop()
Ivan
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Nikolay Aleksandrov @ 2017-05-19 15:03 UTC (permalink / raw)
To: Florian Westphal; +Cc: Xin Long, network dev, davem, cera
In-Reply-To: <20170519144823.GF28091@breakpoint.cc>
On 5/19/17 5:48 PM, Florian Westphal wrote:
> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>> On 5/19/17 5:20 PM, Xin Long wrote:
>>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>> kernel hello and hold timers"), bridge would not start hello_timer if
>>> stp_enabled is not KERNEL_STP when br_dev_open.
>>>
>>> The problem is even if users set stp_enabled with KERNEL_STP later,
>>> the timer will still not be started. It causes that KERNEL_STP can
>>> not really work. Users have to re-ifup the bridge to avoid this.
>>>
>>> This patch is to fix it by starting br->hello_timer when enabling
>>> KERNEL_STP in br_stp_start.
>>>
>>> As an improvement, it's also to start hello_timer again only when
>>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>>> no reason to start the timer again when it's NO_STP.
>>>
>>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and hold timers")
>>> Reported-by: Haidong Li <haili@redhat.com>
>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>> ---
>>> net/bridge/br_stp_if.c | 1 +
>>> net/bridge/br_stp_timer.c | 2 +-
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>
>> This doesn't make much sense to me, how do you change from USER_STP to
>> KERNEL_STP without first going through NO_STP ?
>
> This is easily rerpoduceable via:
>
> ip link add vethin1 type veth peer name vethout1
> ip link add vethin2 type veth peer name vethout2
>
> ip link set vethin1 up
> ip link set vethin2 up
>
> ip link set vethout1 up
> ip link set vethout2 up
>
> brctl addbr br0
> brctl addbr br1
>
> brctl stp br0 on
> brctl stp br1 on
I think this step with moving NO_STP -> KERNEL_STP should be last, then
I can see how the timer won't be started.
>
> brctl addif br0 vethin1
> brctl addif br0 vethin2
>
> brctl addif br1 vethout1
> brctl addif br1 vethout2
>
> ip link set br0 up
> ip link set br1 up
>
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Nikolay Aleksandrov @ 2017-05-19 14:57 UTC (permalink / raw)
To: ivan.vecera; +Cc: Xin Long, network dev, David Miller
In-Reply-To: <CA+Nub0YKgM7dMDYdi43sO2znVtCzToq0dAr5TFEb4Jmw+6zw+w@mail.gmail.com>
On 5/19/17 5:51 PM, Ivan Vecera wrote:
> 2017-05-19 16:45 GMT+02:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
>> On 5/19/17 5:20 PM, Xin Long wrote:
>>>
>>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>>> kernel hello and hold timers"), bridge would not start hello_timer if
>>> stp_enabled is not KERNEL_STP when br_dev_open.
>>>
>>> The problem is even if users set stp_enabled with KERNEL_STP later,
>>> the timer will still not be started. It causes that KERNEL_STP can
>>> not really work. Users have to re-ifup the bridge to avoid this.
>>>
>>> This patch is to fix it by starting br->hello_timer when enabling
>>> KERNEL_STP in br_stp_start.
>>>
>>> As an improvement, it's also to start hello_timer again only when
>>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>>> no reason to start the timer again when it's NO_STP.
>>>
>>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel
>>> hello and hold timers")
>>> Reported-by: Haidong Li <haili@redhat.com>
>>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>>> ---
>>> net/bridge/br_stp_if.c | 1 +
>>> net/bridge/br_stp_timer.c | 2 +-
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>>
>>
>> This doesn't make much sense to me, how do you change from USER_STP to
>> KERNEL_STP without first going through NO_STP ?
>>
>> If you go through NO_STP then all will be fine because br_stp_stop will
>> restart
>> the timers if the previous val was USER_STP.
>>
> The problem occurs when KERNEL_STP is enabled if the bridge itself is already
> up. Then the hello_timer is not started. If the hello and hold timers
> should run only
> when KERNEL_STP is used then there are another problematic places
> (will send follow-up).
>
> Ivan
>
Oh, the problem seems to be rather going from NO_STP -> KERNEL_STP only
then, because you cannot do direct USER_STP -> KERNEL_STP.
^ permalink raw reply
* Re: [PATCH v2 1/3] bpf: Use 1<<16 as ceiling for immediate alignment in verifier.
From: Alexei Starovoitov @ 2017-05-19 14:55 UTC (permalink / raw)
To: Edward Cree, David Miller, Daniel Borkmann; +Cc: alexei.starovoitov, netdev
In-Reply-To: <7402a02c-cb00-d48e-1d2a-ccb03356fb72@solarflare.com>
On 5/19/17 7:21 AM, Edward Cree wrote:
> I'm currently translating the algos to C. But for the kernel patch,
> I'll need to read & understand the existing verifier code, so it
> might take a while :) (I don't suppose there's any design document
> or hacking-notes you could point me at?)
Dave just gave a good overview of the verifier:
https://www.spinics.net/lists/xdp-newbies/msg00185.html
Few more details in
Documentation/networking/filter.txt
and in top comments in kernel/bpf/verifier.c
General bpf arch overview:
http://docs.cilium.io/en/latest/bpf/#instruction-set
> But I'll give it a go for sure.
Thanks!
^ permalink raw reply
* Re: [PATCH v4 net-next 6/7] net: allow simultaneous SW and HW transmit timestamping
From: Willem de Bruijn @ 2017-05-19 14:52 UTC (permalink / raw)
To: Miroslav Lichvar; +Cc: Network Development, Richard Cochran, Willem de Bruijn
In-Reply-To: <20170519100040.GB21003@localhost>
On Fri, May 19, 2017 at 6:00 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> On Thu, May 18, 2017 at 04:16:26PM -0400, Willem de Bruijn wrote:
>> On Thu, May 18, 2017 at 9:06 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
>> > +/* On transmit, software and hardware timestamps are returned independently.
>> > + * As the two skb clones share the hardware timestamp, which may be updated
>> > + * before the software timestamp is received, a hardware TX timestamp may be
>> > + * returned only if there is no software TX timestamp. A false software
>> > + * timestamp made for SOCK_RCVTSTAMP when a real timestamp is missing must
>> > + * be ignored.
>>
>> Please expand on why this case can be ignored. It is quite subtle. How about
>> something like
>>
>> *
>> * A false software timestamp is one made inside the __sock_recv_timestamp
>> * call itself. These are generated whenever SO_TIMESTAMP(NS) is enabled
>> * on the socket, even when the timestamp reported is for another option, such
>> * as hardware tx timestamp.
>> *
>> * Ignore these when deciding whether a timestamp source is hw or sw.
>> */
>
> That seems a bit too verbose to me. :) Would the following work?
>
> /* On transmit, software and hardware timestamps are returned independently.
> * As the two skb clones share the hardware timestamp, which may be updated
> * before the software timestamp is received, a hardware TX timestamp may be
> * returned only if there is no software TX timestamp. Ignore false software
> * timestamps, which may be made in the __sock_recv_timestamp() call when the
> * option SO_TIMESTAMP(NS) is enabled on the socket, even when the skb has a
> * hardware timestamp.
> */
Looks great, thanks.
>
>> > +static bool skb_is_swtx_tstamp(const struct sk_buff *skb,
>> > + const struct sock *sk, int false_tstamp)
>> > +{
>> > + if (false_tstamp && sk->sk_tsflags & SOF_TIMESTAMPING_OPT_TX_SWHW)
>>
>> Also, why is it ignored only for the new mode?
>
> Good point. That should not be there. The function can be now reduced
> to a single line again. I originally tried a different approach,
> disabling false timestamps in the new mode, but then I thought it's
> better to not complicate it unnecessarily and keep it consistent.
>
> --
> Miroslav Lichvar
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Ivan Vecera @ 2017-05-19 14:51 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Xin Long, network dev, David Miller
In-Reply-To: <cf69d5e5-1acf-2ed4-b6cc-fdb1236e7a66@cumulusnetworks.com>
2017-05-19 16:45 GMT+02:00 Nikolay Aleksandrov <nikolay@cumulusnetworks.com>:
> On 5/19/17 5:20 PM, Xin Long wrote:
>>
>> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
>> kernel hello and hold timers"), bridge would not start hello_timer if
>> stp_enabled is not KERNEL_STP when br_dev_open.
>>
>> The problem is even if users set stp_enabled with KERNEL_STP later,
>> the timer will still not be started. It causes that KERNEL_STP can
>> not really work. Users have to re-ifup the bridge to avoid this.
>>
>> This patch is to fix it by starting br->hello_timer when enabling
>> KERNEL_STP in br_stp_start.
>>
>> As an improvement, it's also to start hello_timer again only when
>> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
>> no reason to start the timer again when it's NO_STP.
>>
>> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel
>> hello and hold timers")
>> Reported-by: Haidong Li <haili@redhat.com>
>> Signed-off-by: Xin Long <lucien.xin@gmail.com>
>> ---
>> net/bridge/br_stp_if.c | 1 +
>> net/bridge/br_stp_timer.c | 2 +-
>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>
>
> This doesn't make much sense to me, how do you change from USER_STP to
> KERNEL_STP without first going through NO_STP ?
>
> If you go through NO_STP then all will be fine because br_stp_stop will
> restart
> the timers if the previous val was USER_STP.
>
The problem occurs when KERNEL_STP is enabled if the bridge itself is already
up. Then the hello_timer is not started. If the hello and hold timers
should run only
when KERNEL_STP is used then there are another problematic places
(will send follow-up).
Ivan
^ permalink raw reply
* Re: general protection fault in skb_release_data
From: Andrey Konovalov @ 2017-05-19 14:50 UTC (permalink / raw)
To: Eric Dumazet
Cc: idaifish, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Cong Wang,
syzkaller, Dmitry Vyukov, Kostya Serebryany
In-Reply-To: <CANn89iJ4GJfrJvdzpMesGkUvYzJfUnMwbyPOzDxZdQDAJbkptg@mail.gmail.com>
On Fri, May 19, 2017 at 4:36 PM, 'Eric Dumazet' via syzkaller
<syzkaller@googlegroups.com> wrote:
> On Fri, May 19, 2017 at 5:57 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
>> On Fri, May 19, 2017 at 12:18 PM, <idaifish@gmail.com> wrote:
>>> Hi,
>>>
>>> I've got the following bug report while fuzzing the
>>> kernel(master-f83246089ca) with syzkaller.
>>>
>>> program and config are attached.
>>
>> Hi!
>>
>> Thanks for the report!
>>
>> Adding kernel maintainers.
>>
>> I can confirm that we've hist this bug multiple times, but never been
>> able to reproduce it.
>>
>> I was able to reproduce it on 2ea659a9ef488125eb46da6eb571de5eae5c43f6
>> (4.12-rc1).
>>
>> Using the attached syzkaller program I was able to generate C
>> reproducer, attached. Sometimes I need to run it a few times to
>> trigger the bug.
>>
>> @idaifish If you find more bugs please run ./scripts/get_maintainer.pl
>> to get the list of subsystem maintainers and add them to the
>> recipients. I've updated instructions of how to report kernel bugs
>> found with syzkaller in README.
>>
>> Thanks!
>>
>>>
>>> =======================================================
>>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>>> general protection fault: 0000 [#1] SMP KASAN
>>> Dumping ftrace buffer:
>>> (ftrace buffer empty)
>>> Modules linked in:
>>> CPU: 2 PID: 21599 Comm: syz-executor3 Not tainted 4.11.0-rc8+ #1
>>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>>> Ubuntu-1.8.2-1ubuntu1 04/01/2014
>>> task: ffff88006c16dec0 task.stack: ffff880058bb8000
>>> RIP: 0010:__read_once_size include/linux/compiler.h:254 [inline]
>>> RIP: 0010:compound_head include/linux/page-flags.h:146 [inline]
>>> RIP: 0010:put_page include/linux/mm.h:796 [inline]
>>> RIP: 0010:__skb_frag_unref include/linux/skbuff.h:2613 [inline]
>>> RIP: 0010:skb_release_data+0x201/0x3b0 net/core/skbuff.c:593
>>> RSP: 0018:ffff880058bbf570 EFLAGS: 00010a02
>>> RAX: 11032b488bad1523 RBX: ffff88006c6e8ec8 RCX: ffffc9000190e000
>>> RDX: 1ffff1000d8dd1df RSI: ffffffff8293d7e3 RDI: 88195a445d68a919
>>> RBP: ffff880058bbf5a8 R08: 7bdf27567b31597f R09: 0000000000000000
>>> R10: 00000000000000d2 R11: 7dc9ab6dec891f24 R12: 0000000000000000
>>> R13: dffffc0000000000 R14: ffff88006a475940 R15: 88195a445d68a8f9
>>> FS: 00007fbbc6a34700(0000) GS:ffff88006e400000(0000) knlGS:0000000000000000
>>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: 000000002004e000 CR3: 000000006b868000 CR4: 00000000000006e0
>>> Call Trace:
>>> skb_release_all+0x4a/0x60 net/core/skbuff.c:669
>>> __kfree_skb net/core/skbuff.c:683 [inline]
>>> kfree_skb+0x85/0x1b0 net/core/skbuff.c:704
>>> __ip6_append_data.isra.42+0x26ed/0x33b0 net/ipv6/ip6_output.c:1519
>>> ip6_append_data+0x1a8/0x2f0 net/ipv6/ip6_output.c:1633
>>> udpv6_sendmsg+0x7bd/0x2360 net/ipv6/udp.c:1264
>>> inet_sendmsg+0x123/0x3a0 net/ipv4/af_inet.c:762
>>> sock_sendmsg_nosec net/socket.c:633 [inline]
>>> sock_sendmsg+0xca/0x110 net/socket.c:643
>>> ___sys_sendmsg+0x79f/0x900 net/socket.c:1997
>>> __sys_sendmsg+0xd1/0x170 net/socket.c:2031
>>> SYSC_sendmsg net/socket.c:2042 [inline]
>>> SyS_sendmsg+0x2d/0x50 net/socket.c:2038
>>> entry_SYSCALL_64_fastpath+0x1a/0xa9
>>> RIP: 0033:0x44fb79
>>> RSP: 002b:00007fbbc6a33b58 EFLAGS: 00000212 ORIG_RAX: 000000000000002e
>>> RAX: ffffffffffffffda RBX: 0000000000718000 RCX: 000000000044fb79
>>> RDX: 0000000000000000 RSI: 000000002000afc8 RDI: 0000000000000005
>>> RBP: 00000000000003fb R08: 0000000000000000 R09: 0000000000000000
>>> R10: 0000000000000000 R11: 0000000000000212 R12: 0000000000000005
>>> R13: 0000000020001000 R14: 0000000000048000 R15: 0000000000000000
>>> Code: 48 83 c0 03 48 c1 e0 04 48 01 d8 48 89 c2 48 c1 ea 03 42 80 3c 2a 00
>>> 0f 85 92 01 00 00 4c 8b 38 49 8d 7f 20 48 89 f8 48 c1 e8 03 <42> 80 3c 28 00
>>> 0f 85 6f 01 00 00 49 8b 47 20 a8 01 0f 84 3b ff
>>> RIP: __read_once_size include/linux/compiler.h:254 [inline] RSP:
>>> ffff880058bbf570
>>> RIP: compound_head include/linux/page-flags.h:146 [inline] RSP:
>>> ffff880058bbf570
>>> RIP: put_page include/linux/mm.h:796 [inline] RSP: ffff880058bbf570
>>> RIP: __skb_frag_unref include/linux/skbuff.h:2613 [inline] RSP:
>>> ffff880058bbf570
>>> RIP: skb_release_data+0x201/0x3b0 net/core/skbuff.c:593 RSP:
>>> ffff880058bbf570
>>> ---[ end trace 46c9f72a66cd8627 ]---
>>> Kernel panic - not syncing: Fatal exception
>>> Dumping ftrace buffer:
>>> (ftrace buffer empty)
>>> Kernel Offset: disabled
>>> Rebooting in 86400 seconds..
>>>
>>> --
>>> You received this message because you are subscribed to the Google Groups
>>> "syzkaller" group.
>>> To unsubscribe from this group and stop receiving emails from it, send an
>>> email to syzkaller+unsubscribe@googlegroups.com.
>>> For more options, visit https://groups.google.com/d/optout.
>
> Hi Andrey, please try following patch
> Thanks.
Hi Eric,
Your patch fixes the bug for me.
Thanks!
Tested-by: Andrey Konovalov <andreyknvl@google.com>
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Florian Westphal @ 2017-05-19 14:48 UTC (permalink / raw)
To: Nikolay Aleksandrov; +Cc: Xin Long, network dev, davem, cera
In-Reply-To: <cf69d5e5-1acf-2ed4-b6cc-fdb1236e7a66@cumulusnetworks.com>
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
> On 5/19/17 5:20 PM, Xin Long wrote:
> >Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
> >kernel hello and hold timers"), bridge would not start hello_timer if
> >stp_enabled is not KERNEL_STP when br_dev_open.
> >
> >The problem is even if users set stp_enabled with KERNEL_STP later,
> >the timer will still not be started. It causes that KERNEL_STP can
> >not really work. Users have to re-ifup the bridge to avoid this.
> >
> >This patch is to fix it by starting br->hello_timer when enabling
> >KERNEL_STP in br_stp_start.
> >
> >As an improvement, it's also to start hello_timer again only when
> >br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
> >no reason to start the timer again when it's NO_STP.
> >
> >Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and hold timers")
> >Reported-by: Haidong Li <haili@redhat.com>
> >Signed-off-by: Xin Long <lucien.xin@gmail.com>
> >---
> > net/bridge/br_stp_if.c | 1 +
> > net/bridge/br_stp_timer.c | 2 +-
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
>
> This doesn't make much sense to me, how do you change from USER_STP to
> KERNEL_STP without first going through NO_STP ?
This is easily rerpoduceable via:
ip link add vethin1 type veth peer name vethout1
ip link add vethin2 type veth peer name vethout2
ip link set vethin1 up
ip link set vethin2 up
ip link set vethout1 up
ip link set vethout2 up
brctl addbr br0
brctl addbr br1
brctl stp br0 on
brctl stp br1 on
brctl addif br0 vethin1
brctl addif br0 vethin2
brctl addif br1 vethout1
brctl addif br1 vethout2
ip link set br0 up
ip link set br1 up
^ permalink raw reply
* RE: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low
From: David Laight @ 2017-05-19 14:46 UTC (permalink / raw)
To: 'Bjørn Mork'
Cc: 'Jim Baxter', linux-usb@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Oliver Neukum
In-Reply-To: <87pof5rlw4.fsf@miraculix.mork.no>
From: Bjørn Mork
> Sent: 19 May 2017 14:56
...
> Unless someone has a nice way to just collect a list of skbs and have
> them converted to proper framing on the fly when transmitting, without
> having to care about USB packet boundaries.
skb can be linked into arbitrary chains (or even trees), but I suspect
the usbnet code would need to be taught about them.
For XHCI it isn't too bad because it will do arbitrary scatter-gather
(apart from the ring end).
But I believe the earlier controllers only support fragments that
end on usb packet boundaries.
I looked at the usbnet code a few years ago, I'm sure it ought to
be possible to shortcut most of the code that uses URB and directly
write to the xhci (in particular) ring descriptors.
For receive you probably want to feed the USB stack multiple (probably)
2k buffers, and handle the debatching into ethernet frames yourself.
One of the ASIX drivers used to be particularly bad, it allocated 64k
skb for receive (the hardware can merge IP packets) and then hacked
the true_size before passing upstream.
David
^ permalink raw reply
* Re: [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Nikolay Aleksandrov @ 2017-05-19 14:45 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, cera
In-Reply-To: <6e78bfea3969ca1a937b37e28e796a3047b82615.1495203629.git.lucien.xin@gmail.com>
On 5/19/17 5:20 PM, Xin Long wrote:
> Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
> kernel hello and hold timers"), bridge would not start hello_timer if
> stp_enabled is not KERNEL_STP when br_dev_open.
>
> The problem is even if users set stp_enabled with KERNEL_STP later,
> the timer will still not be started. It causes that KERNEL_STP can
> not really work. Users have to re-ifup the bridge to avoid this.
>
> This patch is to fix it by starting br->hello_timer when enabling
> KERNEL_STP in br_stp_start.
>
> As an improvement, it's also to start hello_timer again only when
> br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
> no reason to start the timer again when it's NO_STP.
>
> Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and hold timers")
> Reported-by: Haidong Li <haili@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/bridge/br_stp_if.c | 1 +
> net/bridge/br_stp_timer.c | 2 +-
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
This doesn't make much sense to me, how do you change from USER_STP to
KERNEL_STP without first going through NO_STP ?
If you go through NO_STP then all will be fine because br_stp_stop will restart
the timers if the previous val was USER_STP.
^ permalink raw reply
* Re: general protection fault in skb_release_data
From: Eric Dumazet @ 2017-05-19 14:36 UTC (permalink / raw)
To: Andrey Konovalov
Cc: idaifish, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Cong Wang,
syzkaller, Dmitry Vyukov, Kostya Serebryany
In-Reply-To: <CAAeHK+wet1o-F+-Q5jEh9DigDwouERjvrh1wQ1kFBxA5geGJpg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4674 bytes --]
On Fri, May 19, 2017 at 5:57 AM, Andrey Konovalov <andreyknvl@google.com> wrote:
> On Fri, May 19, 2017 at 12:18 PM, <idaifish@gmail.com> wrote:
>> Hi,
>>
>> I've got the following bug report while fuzzing the
>> kernel(master-f83246089ca) with syzkaller.
>>
>> program and config are attached.
>
> Hi!
>
> Thanks for the report!
>
> Adding kernel maintainers.
>
> I can confirm that we've hist this bug multiple times, but never been
> able to reproduce it.
>
> I was able to reproduce it on 2ea659a9ef488125eb46da6eb571de5eae5c43f6
> (4.12-rc1).
>
> Using the attached syzkaller program I was able to generate C
> reproducer, attached. Sometimes I need to run it a few times to
> trigger the bug.
>
> @idaifish If you find more bugs please run ./scripts/get_maintainer.pl
> to get the list of subsystem maintainers and add them to the
> recipients. I've updated instructions of how to report kernel bugs
> found with syzkaller in README.
>
> Thanks!
>
>>
>> =======================================================
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault: 0000 [#1] SMP KASAN
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Modules linked in:
>> CPU: 2 PID: 21599 Comm: syz-executor3 Not tainted 4.11.0-rc8+ #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
>> Ubuntu-1.8.2-1ubuntu1 04/01/2014
>> task: ffff88006c16dec0 task.stack: ffff880058bb8000
>> RIP: 0010:__read_once_size include/linux/compiler.h:254 [inline]
>> RIP: 0010:compound_head include/linux/page-flags.h:146 [inline]
>> RIP: 0010:put_page include/linux/mm.h:796 [inline]
>> RIP: 0010:__skb_frag_unref include/linux/skbuff.h:2613 [inline]
>> RIP: 0010:skb_release_data+0x201/0x3b0 net/core/skbuff.c:593
>> RSP: 0018:ffff880058bbf570 EFLAGS: 00010a02
>> RAX: 11032b488bad1523 RBX: ffff88006c6e8ec8 RCX: ffffc9000190e000
>> RDX: 1ffff1000d8dd1df RSI: ffffffff8293d7e3 RDI: 88195a445d68a919
>> RBP: ffff880058bbf5a8 R08: 7bdf27567b31597f R09: 0000000000000000
>> R10: 00000000000000d2 R11: 7dc9ab6dec891f24 R12: 0000000000000000
>> R13: dffffc0000000000 R14: ffff88006a475940 R15: 88195a445d68a8f9
>> FS: 00007fbbc6a34700(0000) GS:ffff88006e400000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 000000002004e000 CR3: 000000006b868000 CR4: 00000000000006e0
>> Call Trace:
>> skb_release_all+0x4a/0x60 net/core/skbuff.c:669
>> __kfree_skb net/core/skbuff.c:683 [inline]
>> kfree_skb+0x85/0x1b0 net/core/skbuff.c:704
>> __ip6_append_data.isra.42+0x26ed/0x33b0 net/ipv6/ip6_output.c:1519
>> ip6_append_data+0x1a8/0x2f0 net/ipv6/ip6_output.c:1633
>> udpv6_sendmsg+0x7bd/0x2360 net/ipv6/udp.c:1264
>> inet_sendmsg+0x123/0x3a0 net/ipv4/af_inet.c:762
>> sock_sendmsg_nosec net/socket.c:633 [inline]
>> sock_sendmsg+0xca/0x110 net/socket.c:643
>> ___sys_sendmsg+0x79f/0x900 net/socket.c:1997
>> __sys_sendmsg+0xd1/0x170 net/socket.c:2031
>> SYSC_sendmsg net/socket.c:2042 [inline]
>> SyS_sendmsg+0x2d/0x50 net/socket.c:2038
>> entry_SYSCALL_64_fastpath+0x1a/0xa9
>> RIP: 0033:0x44fb79
>> RSP: 002b:00007fbbc6a33b58 EFLAGS: 00000212 ORIG_RAX: 000000000000002e
>> RAX: ffffffffffffffda RBX: 0000000000718000 RCX: 000000000044fb79
>> RDX: 0000000000000000 RSI: 000000002000afc8 RDI: 0000000000000005
>> RBP: 00000000000003fb R08: 0000000000000000 R09: 0000000000000000
>> R10: 0000000000000000 R11: 0000000000000212 R12: 0000000000000005
>> R13: 0000000020001000 R14: 0000000000048000 R15: 0000000000000000
>> Code: 48 83 c0 03 48 c1 e0 04 48 01 d8 48 89 c2 48 c1 ea 03 42 80 3c 2a 00
>> 0f 85 92 01 00 00 4c 8b 38 49 8d 7f 20 48 89 f8 48 c1 e8 03 <42> 80 3c 28 00
>> 0f 85 6f 01 00 00 49 8b 47 20 a8 01 0f 84 3b ff
>> RIP: __read_once_size include/linux/compiler.h:254 [inline] RSP:
>> ffff880058bbf570
>> RIP: compound_head include/linux/page-flags.h:146 [inline] RSP:
>> ffff880058bbf570
>> RIP: put_page include/linux/mm.h:796 [inline] RSP: ffff880058bbf570
>> RIP: __skb_frag_unref include/linux/skbuff.h:2613 [inline] RSP:
>> ffff880058bbf570
>> RIP: skb_release_data+0x201/0x3b0 net/core/skbuff.c:593 RSP:
>> ffff880058bbf570
>> ---[ end trace 46c9f72a66cd8627 ]---
>> Kernel panic - not syncing: Fatal exception
>> Dumping ftrace buffer:
>> (ftrace buffer empty)
>> Kernel Offset: disabled
>> Rebooting in 86400 seconds..
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "syzkaller" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to syzkaller+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/d/optout.
Hi Andrey, please try following patch
Thanks.
[-- Attachment #2: patch2482 --]
[-- Type: application/octet-stream, Size: 1123 bytes --]
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d4a31becbd25dda895d7391e1e65c2de237bf2a3..f66ecd9cebab7619ebe82e0efd78ff386e2b35a6 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -1506,6 +1506,13 @@ static int __ip6_append_data(struct sock *sk,
data += fragheaderlen;
skb->transport_header = (skb->network_header +
fragheaderlen);
+ copy = datalen - transhdrlen - fraggap;
+
+ if (copy < 0) {
+ err = -EINVAL;
+ kfree_skb(skb);
+ goto error;
+ }
if (fraggap) {
skb->csum = skb_copy_and_csum_bits(
skb_prev, maxfraglen,
@@ -1515,13 +1522,9 @@ static int __ip6_append_data(struct sock *sk,
data += fraggap;
pskb_trim_unique(skb_prev, maxfraglen);
}
- copy = datalen - transhdrlen - fraggap;
-
- if (copy < 0) {
- err = -EINVAL;
- kfree_skb(skb);
- goto error;
- } else if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) {
+ if (copy > 0 && getfrag(from, data + transhdrlen,
+ offset, copy, fraggap,
+ skb) < 0) {
err = -EFAULT;
kfree_skb(skb);
goto error;
^ permalink raw reply related
* Re: [PATCH v2 1/3] bpf: Use 1<<16 as ceiling for immediate alignment in verifier.
From: Edward Cree @ 2017-05-19 14:21 UTC (permalink / raw)
To: Alexei Starovoitov, David Miller, Daniel Borkmann
Cc: alexei.starovoitov, netdev
In-Reply-To: <17a1c4a2-68d7-0f8f-f009-b97706d67ceb@fb.com>
On 19/05/17 02:22, Alexei Starovoitov wrote:
> In your .py I'd only change __str__(self) to print them in mask,value
> as the order they're passed into constructor to make it easier to read.
Actually I was going to go the other way and change the ctor to take
value,mask. But I agree they're inconsistent right now.
> this mul algo I don't completely understand. It feels correct,
> but I'm not sure we really need it for the kernel.
You're probably right; I was just driven by a completionist desire to
cover everything I could.
> What I love about the whole thing that it works for access into
> packet, access into map values and in the future for any other
> variable length access.
Sure, but don't start thinking it subsumes all the other checks. We
will still need e.g. max/min tracking, because packet length isn't
always a power of 2.
> Are you planning to work on the kernel patch for this algo?
> Once we have it the verifier will be smarter regarding
> alignment tracking than any compiler i know :)
I'm currently translating the algos to C. But for the kernel patch,
I'll need to read & understand the existing verifier code, so it
might take a while :) (I don't suppose there's any design document
or hacking-notes you could point me at?)
But I'll give it a go for sure.
-Ed
^ permalink raw reply
* [PATCH net] bridge: start hello_timer when enabling KERNEL_STP in br_stp_start
From: Xin Long @ 2017-05-19 14:20 UTC (permalink / raw)
To: network dev; +Cc: davem, nikolay, cera
Since commit 76b91c32dd86 ("bridge: stp: when using userspace stp stop
kernel hello and hold timers"), bridge would not start hello_timer if
stp_enabled is not KERNEL_STP when br_dev_open.
The problem is even if users set stp_enabled with KERNEL_STP later,
the timer will still not be started. It causes that KERNEL_STP can
not really work. Users have to re-ifup the bridge to avoid this.
This patch is to fix it by starting br->hello_timer when enabling
KERNEL_STP in br_stp_start.
As an improvement, it's also to start hello_timer again only when
br->stp_enabled is KERNEL_STP in br_hello_timer_expired, there is
no reason to start the timer again when it's NO_STP.
Fixes: 76b91c32dd86 ("bridge: stp: when using userspace stp stop kernel hello and hold timers")
Reported-by: Haidong Li <haili@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/bridge/br_stp_if.c | 1 +
net/bridge/br_stp_timer.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c
index 08341d2..0db8102 100644
--- a/net/bridge/br_stp_if.c
+++ b/net/bridge/br_stp_if.c
@@ -179,6 +179,7 @@ static void br_stp_start(struct net_bridge *br)
br_debug(br, "using kernel STP\n");
/* To start timers on any ports left in blocking */
+ mod_timer(&br->hello_timer, jiffies + br->hello_time);
br_port_state_selection(br);
}
diff --git a/net/bridge/br_stp_timer.c b/net/bridge/br_stp_timer.c
index c98b3e5..60b6fe2 100644
--- a/net/bridge/br_stp_timer.c
+++ b/net/bridge/br_stp_timer.c
@@ -40,7 +40,7 @@ static void br_hello_timer_expired(unsigned long arg)
if (br->dev->flags & IFF_UP) {
br_config_bpdu_generation(br);
- if (br->stp_enabled != BR_USER_STP)
+ if (br->stp_enabled == BR_KERNEL_STP)
mod_timer(&br->hello_timer,
round_jiffies(jiffies + br->hello_time));
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net 3/3] virtio-net: enable TSO/checksum offloads for Q-in-Q vlans
From: Jason Wang @ 2017-05-19 14:18 UTC (permalink / raw)
To: Vladislav Yasevich, netdev
Cc: Vladislav Yasevich, Michael S. Tsirkin, virtualization
In-Reply-To: <1495114265-23368-4-git-send-email-vyasevic@redhat.com>
On 2017年05月18日 21:31, Vladislav Yasevich wrote:
> Since virtio does not provide it's own ndo_features_check handler,
> TSO, and now checksum offload, are disabled for stacked vlans.
> Re-enable the support and let the host take care of it. This
> restores/improves Guest-to-Guest performance over Q-in-Q vlans.
>
> CC: "Michael S. Tsirkin" <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> CC: virtualization@lists.linux-foundation.org
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
> ---
> drivers/net/virtio_net.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 8324a5e..341fb96 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -2028,6 +2028,7 @@ static const struct net_device_ops virtnet_netdev = {
> .ndo_poll_controller = virtnet_netpoll,
> #endif
> .ndo_xdp = virtnet_xdp,
> + .ndo_features_check = passthru_features_check,
> };
>
> static void virtnet_config_changed_work(struct work_struct *work)
Acked-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* RE: [PATCH net-next] cxgb4: fix incorrect cim_la output for T6
From: David Laight @ 2017-05-19 14:17 UTC (permalink / raw)
To: 'Ganesh Goudar', netdev@vger.kernel.org,
davem@davemloft.net
Cc: nirranjan@chelsio.com, indranil@chelsio.com
In-Reply-To: <1495188738-20546-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar
> Sent: 19 May 2017 11:12
T6
>
> take care of UpDbgLaRdPtr[0-3] restriction for T6
>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> ---
> drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> index aded42b96..917b46b 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
> @@ -8268,6 +8268,13 @@ int t4_cim_read_la(struct adapter *adap, u32 *la_buf, unsigned int *wrptr)
> if (ret)
> break;
> idx = (idx + 1) & UPDBGLARDPTR_M;
> +
> + /* Bits 0-3 of UpDbgLaRdPtr can be between 0000 to 1001 to
> + * identify the 32-bit portion of the full 312-bit data
> + */
> + if (is_t6(adap->params.chip))
> + while ((idx & 0xf) > 9)
> + idx = (idx + 1) % UPDBGLARDPTR_M;
Why the loop, maybe:
if (is_t6(adap->params.chip) && (idx & 0xf) >= 9)
idx = (idx & 0xf0) + 0x10;
else
idx++;
idx &= UPDBGLARDPTR_M;
David
^ permalink raw reply
* Good day my dear, please i need your assistant
From: Aisha Muammar Gaddafi @ 2017-05-19 14:09 UTC (permalink / raw)
In-Reply-To: <1326458444.1869483.1495202943669.ref@mail.yahoo.com>
How are you doing today, Hope you are in perfect condition? Please i apologies with you to exercise a little patience and read through my letter i am contacting you for obvious reason and important business. My Name is Aisha Gaddafi,i am open minded person and easy going person romance and hard working, Please i really need to have a good relationship with you, i have some important discussion to discuss with you.There is information i would like you to keep very confidential, i will appreciate if you can reply me through my Private email address (aishamuammarg03@yahoo.com) for further discussion, I WILL BE VERY HAPPY IF YOU ACCEPT ME AS YOUR FRIEND,Thanks and God bless, this is just brief about me,hope to review the secret inner most part of me once i received from you.
Aisha Gaddafi.
^ permalink raw reply
* [PATCH v2 net] smsc95xx: Support only IPv4 TCP/UDP csum offload
From: Nisar.Sayed @ 2017-05-19 14:00 UTC (permalink / raw)
To: netdev, davem; +Cc: UNGLinuxDriver, steve.glendinning, popcornmix
From: Nisar Sayed <Nisar.Sayed@microchip.com>
When TX checksum offload is used, if the computed checksum is 0 the
LAN95xx device do not alter the checksum to 0xffff. In the case of ipv4
UDP checksum, it indicates to receiver that no checksum is calculated.
Under ipv6, UDP checksum yields a result of zero must be changed to
0xffff. Hence disabling checksum offload for ipv6 packets.
Signed-off-by: Nisar Sayed <Nisar.Sayed@microchip.com>
Reported-by: popcorn mix <popcornmix@gmail.com>
---
Changes
V2
- Updated e-mail alias
- Updated commit message and comments
drivers/net/usb/smsc95xx.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
index 765400b..2dfca96 100644
--- a/drivers/net/usb/smsc95xx.c
+++ b/drivers/net/usb/smsc95xx.c
@@ -681,7 +681,7 @@ static int smsc95xx_set_features(struct net_device *netdev,
if (ret < 0)
return ret;
- if (features & NETIF_F_HW_CSUM)
+ if (features & NETIF_F_IP_CSUM)
read_buf |= Tx_COE_EN_;
else
read_buf &= ~Tx_COE_EN_;
@@ -1279,12 +1279,19 @@ static int smsc95xx_bind(struct usbnet *dev, struct usb_interface *intf)
spin_lock_init(&pdata->mac_cr_lock);
+ /* LAN95xx devices do not alter the computed checksum of 0 to 0xffff.
+ * RFC 2460, ipv6 UDP calculated checksum yields a result of zero must
+ * be changed to 0xffff. RFC 768, ipv4 UDP computed checksum is zero,
+ * it is transmitted as all ones. The zero transmitted checksum means
+ * transmitter generated no checksum. Hence, enable csum offload only
+ * for ipv4 packets.
+ */
if (DEFAULT_TX_CSUM_ENABLE)
- dev->net->features |= NETIF_F_HW_CSUM;
+ dev->net->features |= NETIF_F_IP_CSUM;
if (DEFAULT_RX_CSUM_ENABLE)
dev->net->features |= NETIF_F_RXCSUM;
- dev->net->hw_features = NETIF_F_HW_CSUM | NETIF_F_RXCSUM;
+ dev->net->hw_features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM;
smsc95xx_init_mac_address(dev);
--
1.9.1
^ permalink raw reply related
* Re: [RFC V1 1/1] net: cdc_ncm: Reduce memory use when kernel memory low
From: Bjørn Mork @ 2017-05-19 13:55 UTC (permalink / raw)
To: David Laight
Cc: 'Jim Baxter', linux-usb@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Oliver Neukum
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DCFFFAA5C@AcuExch.aculab.com>
David Laight <David.Laight@ACULAB.COM> writes:
> From: linux-usb-owner@vger.kernel.org [mailto:linux-usb-owner@vger.kernel.org] On Behalf Of Jim Baxter
>> Sent: 16 May 2017 18:41
>>
>> The CDC-NCM driver can require large amounts of memory to create
>> skb's and this can be a problem when the memory becomes fragmented.
>>
>> This especially affects embedded systems that have constrained
>> resources but wish to maximise the throughput of CDC-NCM with 16KiB
>> NTB's.
>
> Why is this driver copying multiple tx messages into a single skb.
Mostly becasue it already did that when I started messing with it, and I
didn't know how to avoid that.
> Surely there are better ways to do this??
But I have been there thinking this exact thought a couple of times.
Suggestions are appreciated.
> I think it is generating a 'multi-ethernet frame' URB with an
> overall header for each URB and a header for each ethernet frame.
With some negotiated alignment restrictions, and a linked list of frame
pointer arrays. But yes, that is basically it.
(it's not always ethernet - with MBIM it can be IP or arbitrary as well,
but I don't think that makes any difference)
> Given that the USB stack allows multiple concurrent transmits I'm
> surprised that batching large ethernet frames makes much difference.
Me too. Actually, I don't think it does. The protocol was developed
with specific device restrictions in mind. These might be invalid today.
There is no reason to believe that using simple CDC ECM framing
(i.e. one ethernet frame per URB) is any problem.
> Also the USB target can't actually tell when URB that contain
> multiples of the USB packet size end.
> So it is possible to send a single NTB as multiple URB.
Nice idea! Never thought of that. Yes, the driver could use a number
smaller buffers regardless of the NTB size, by abusing the fact that the
device will see them as a contigious USB transfer as long as they fall
on USB packet boundaries.
Started thinking about how to do this in practice. It seemed obviously
simply to jsut fire off the buffers as they fill up until the the max
aggregation size or time has been exceeded. But the header makes this
harder than necessary. It contains both a length and a pointer to the
first frame pointer array (NDP). So we will have to decide the size of
the NTB and where to put the first NDP before sending the first USB
packet. This is possible if we always go for the pad-to-max strategy.
We'll also have to make some assumptions about the size of the NDP(s) as
we add them, but we already do that so I don't think it is a big deal.
Might be the way to go.
Unless someone has a nice way to just collect a list of skbs and have
them converted to proper framing on the fly when transmitting, without
having to care about USB packet boundaries.
Bjørn
^ permalink raw reply
* Re: [PATCH net 1/3] vlan: Fix tcp checksums offloads for Q-in-Q vlan.
From: Toshiaki Makita @ 2017-05-19 13:31 UTC (permalink / raw)
To: vyasevic, Toshiaki Makita, Vladislav Yasevich, netdev; +Cc: mkubecek
In-Reply-To: <1eeb2dc0-5877-0834-76af-677f6ca6b5d9@redhat.com>
On 17/05/19 (金) 18:53, Vlad Yasevich wrote:
> On 05/19/2017 04:16 AM, Toshiaki Makita wrote:
>> On 2017/05/19 16:09, Vlad Yasevich wrote:
>>> On 05/18/2017 10:13 PM, Toshiaki Makita wrote:
>>>> On 2017/05/18 22:31, Vladislav Yasevich wrote:
>>>>> It appears that since commit 8cb65d000, Q-in-Q vlans have been
>>>>> broken. The series that commit is part of enabled TSO and checksum
>>>>> offloading on Q-in-Q vlans. However, most HW we support can't handle
>>>>> it. To work around the issue, the above commit added a function that
>>>>> turns off offloads on Q-in-Q devices, but it left the checksum offload.
>>>>> That will cause issues with most older devices that supprort very basic
>>>>> checksum offload capabilities as well as some newer devices (we've
>>>>> reproduced te problem with both be2net and bnx).
>>>>>
>>>>> To solve this for everyone, turn off checksum offloading feature
>>>>> by default when sending Q-in-Q traffic. Devices that are proven to
>>>>> work can provided a corrected ndo_features_check implemetation.
>>>>>
>>>>> Fixes: 8cb65d000 ("net: Move check for multiple vlans to drivers")
>>>>> CC: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>>>>> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>>>>> ---
>>>>> include/linux/if_vlan.h | 1 -
>>>>> 1 file changed, 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
>>>>> index 8d5fcd6..ae537f0 100644
>>>>> --- a/include/linux/if_vlan.h
>>>>> +++ b/include/linux/if_vlan.h
>>>>> @@ -619,7 +619,6 @@ static inline netdev_features_t vlan_features_check(const struct sk_buff *skb,
>>>>> NETIF_F_SG |
>>>>> NETIF_F_HIGHDMA |
>>>>> NETIF_F_FRAGLIST |
>>>>> - NETIF_F_HW_CSUM |
>>>>> NETIF_F_HW_VLAN_CTAG_TX |
>>>>> NETIF_F_HW_VLAN_STAG_TX);
>>>>>
>>>>
>>>> I guess HW_CSUM theoretically can handle Q-in-Q packets and the problem
>>>> is IP_CSUM and IPV6_CSUM.
>>>> So wouldn't it be better to leave HW_CSUM and drop IP_CSUM/IPV6_CSUM,
>>>> i.e. change intersection into bitwise AND?
>>>>
>>>
>>> It wasn't really a problem before accelerations got enabled on q-in-q
>>> vlans.
>>
>> Right for stacked vlan device.
>> But I think the check was there for packets from guests forwarded by
>> bridge to vlan device so it was a problem before 8cb65d000.
>
> Not really, since stacked vlans in guests wouldn't have accelerations on.
> Haven't really tried a new guest on old hosts. It might be an issue there...
It's real. I'm now remembering that I came across a similar issue before
introducing 8cb65d000. The situation was that bridge (vlan_filtering)
adds a vlan tag to a frame which is already tagged by guests, or by a
vlan device on the top of the bridge (Note that virtio and bridge have
HW_CSUM in vlan_features). I addressed the problem in drivers side since
all the IP/IPV6_CSUM drivers I encountered the issue on are able to
notify devices of IP header offset. Now I checked be2net driver's code
and realized it doesn't provide IP offset so it makes sense to drop
IP/IPV6_CSUM by default. Anyway, kernels before 8cb65d000 have that
problem, not only after 8cb65d000.
Toshiaki Makita
^ permalink raw reply
* Re: [PATCH net] ah: use crypto_memneq to check the ICV
From: Steffen Klassert @ 2017-05-19 13:24 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: netdev, Herbert Xu
In-Reply-To: <20170519122514.GA3559@bistromath.localdomain>
On Fri, May 19, 2017 at 02:25:14PM +0200, Sabrina Dubroca wrote:
> Hi Steffen,
>
> 2017-05-04, 12:41:24 +0200, Steffen Klassert wrote:
> > On Wed, May 03, 2017 at 04:57:57PM +0200, Sabrina Dubroca wrote:
> > > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
> > > ---
> > > net/ipv4/ah4.c | 5 +++--
> > > net/ipv6/ah6.c | 5 +++--
> > > 2 files changed, 6 insertions(+), 4 deletions(-)
> >
> > Is this a fix for something? If so, please describe what it fixes.
> > If not, it can wait until after the merge window and merged into
> > ipsec-next then.
>
> Do you prefer that I resend the patch, or can you take it directly?
No need to resend, I've just applied it directly.
Thanks a lot Sabrina!
^ 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