* Re: [PATCH net-next 0/8] add TX timestamping via cmsg
From: Willem de Bruijn @ 2016-03-31 3:50 UTC (permalink / raw)
To: Martin KaFai Lau, Soheil Hassas Yeganeh; +Cc: Network Development
In-Reply-To: <CADAM+B1osNNwVpg1R_kAFW8m75mr15_usDa=gmqpBA+xwyA_fg@mail.gmail.com>
>> Nice patches!
This does not yet solve the append issue that your MSG_EOR patch
addresses, of course.
The straightforward jump to new_segment that I proposed as
simplification is more properly a "start-of-record" than
"end-of-record" signal. It is probably preferable to indeed be able to
pass EOR as signal that the last skb must not be appended to in
subsequent calls.
I think that the record bounds issue is best solved independently from
the interface for intermittent timestamps because (a) changing the tcp
bytestream packetization for timestamping introduces subtle
differences between tracked and untracked data that are not always
acceptable and (b) EOR can also be useful outside timestamps. A
zerocopy sendmsg patchset that I sent for RFC last year encountered a
similar requirement, to give one example: each skb with user data must
point to a completion notification structure (ubuf_info), and can only
point to one at a time. Appends that cause a conflict in skb->uarg
pointers had to be blocked, at the cost of possibly different
packetization compared to regular sends.
^ permalink raw reply
* Re: [PATCH net-next 7/8] sock: enable timestamping using control messages
From: Willem de Bruijn @ 2016-03-31 3:39 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-8-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> Currently, SOL_TIMESTAMPING can only be enabled using setsockopt.
> This is very costly when users want to sample writes to gather
> tx timestamps.
>
> Add support for enabling SO_TIMESTAMPING via control messages by
> using tsflags added in `struct sockcm_cookie` (added in the previous
> patches in this series) to set the tx_flags of the last skb created in
> a sendmsg. With this patch, the timestamp recording bits in tx_flags
> of the skbuff is overridden if SO_TIMESTAMPING is passed in a cmsg.
>
> Please note that this is only effective for overriding the recording
> timestamps flags. Users should enable timestamp reporting (e.g.,
> SOF_TIMESTAMPING_SOFTWARE | SOF_TIMESTAMPING_OPT_ID) using
> socket options and then should ask for SOF_TIMESTAMPING_TX_*
> using control messages per sendmsg to sample timestamps for each
> write.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 6/8] ipv6: process socket-level control messages in IPv6
From: Willem de Bruijn @ 2016-03-31 3:37 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-7-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> Process socket-level control messages by invoking
> __sock_cmsg_send in ip6_datagram_send_ctl for control messages on
> the SOL_SOCKET layer.
>
> This makes sure whenever ip6_datagram_send_ctl is called for
> udp and raw, we also process socket-level control messages.
>
> This is a bit uglier than IPv4, since IPv6 does not have
> something like ipcm_cookie. Perhaps we can later create
> a control message cookie for IPv6?
>
> Note that this commit interprets new control messages that
> were ignored before. As such, this commit does not change
> the behavior of IPv6 control messages.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 5/8] ipv4: process socket-level control messages in IPv4
From: Willem de Bruijn @ 2016-03-31 3:36 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-6-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> Process socket-level control messages by invoking
> __sock_cmsg_send in ip_cmsg_send for control messages on
> the SOL_SOCKET layer.
>
> This makes sure whenever ip_cmsg_send is called in udp, icmp,
> and raw, we also process socket-level control messages.
>
> Note that this commit interprets new control messages that
> were ignored before. As such, this commit does not change
> the behavior of IPv4 control messages.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 4/8] sock: accept SO_TIMESTAMPING flags in socket cmsg
From: Willem de Bruijn @ 2016-03-31 3:36 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-5-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> Accept SO_TIMESTAMPING in control messages of the SOL_SOCKET level
> as a basis to accept timestamping requests per write.
>
> This implementation only accepts TX recording flags (i.e.,
> SOF_TIMESTAMPING_TX_HARDWARE, SOF_TIMESTAMPING_TX_SOFTWARE,
> SOF_TIMESTAMPING_TX_SCHED, and SOF_TIMESTAMPING_TX_ACK) in
> control messages. Users need to set reporting flags (e.g.,
> SOF_TIMESTAMPING_OPT_ID) per socket via socket options.
>
> This commit adds a tsflags field in sockcm_cookie which is
> set in __sock_cmsg_send. It only override the SOF_TIMESTAMPING_TX_*
> bits in sockcm_cookie.tsflags allowing the control message
> to override the recording behavior per write, yet maintaining
> the value of other flags.
>
> This patch implements validating the control message and setting
> tsflags in struct sockcm_cookie. Next commits in this series will
> actually implement timestamping per write for different protocols.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 3/8] tcp: use one bit in TCP_SKB_CB to mark ACK timestamps
From: Willem de Bruijn @ 2016-03-31 3:36 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-4-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> Currently, to avoid a cache line miss for accessing skb_shinfo,
> tcp_ack_tstamp skips socket that do not have
> SOF_TIMESTAMPING_TX_ACK bit set in sk_tsflags. This is
> implemented based on an implicit assumption that the
> SOF_TIMESTAMPING_TX_ACK is set via socket options for the
> duration that ACK timestamps are needed.
>
> To implement per-write timestamps, this check should be
> removed and replaced with a per-packet alternative that
> quickly skips packets missing ACK timestamps marks without
> a cache-line miss.
>
> To enable per-packet marking without a cache line miss, use
> one bit in TCP_SKB_CB to mark a whether a SKB might need a
> ack tx timestamp or not. Further checks in tcp_ack_tstamp are not
> modified and work as before.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net-next 2/8] tcp: accept SOF_TIMESTAMPING_OPT_ID for passive TFO
From: Willem de Bruijn @ 2016-03-31 3:35 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: David Miller, Network Development, Willem de Bruijn, Eric Dumazet,
Yuchung Cheng, Neal Cardwell, Martin KaFai Lau,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-3-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 6:37 PM, Soheil Hassas Yeganeh
<soheil.kdev@gmail.com> wrote:
> From: Soheil Hassas Yeganeh <soheil@google.com>
>
> SOF_TIMESTAMPING_OPT_ID is set to get data-independent IDs
> to associate timestamps with send calls. For TCP connections,
> tp->snd_una is used as the starting point to calculate
> relative IDs.
>
> This socket option will fail if set before the handshake on a
> passive TCP fast open connection with data in SYN or SYN/ACK,
> since setsockopt requires the connection to be in the
> ESTABLISHED state.
>
> To address these, instead of limiting the option to the
> ESTABLISHED state, accept the SOF_TIMESTAMPING_OPT_ID option as
> long as the connection is not in LISTEN or CLOSE states.
>
> Signed-off-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* 1oan @ low interest rate
From: Mr. Wolf @ 2016-03-31 1:06 UTC (permalink / raw)
To: Recipients
Are you interested in starting up a business or do you have financial
difficulties that requires capital....what about you over there looking to
solve that personal need? look no further as we are here to offer you a loan of 3% monthly with no hidden charges. All you need to do is send us an email to pierrewoolff@gmail.com for more information
^ permalink raw reply
* Re: [PATCH] net: fec: stop the "rcv is not +last, " error messages
From: Greg Ungerer @ 2016-03-31 1:59 UTC (permalink / raw)
To: Fugang Duan, Fabio Estevam; +Cc: Troy Kisky, netdev@vger.kernel.org
In-Reply-To: <VI1PR0401MB1855458E0B31502D1C112AF6FF990@VI1PR0401MB1855.eurprd04.prod.outlook.com>
Hi Andy,
On 31/03/16 11:41, Fugang Duan wrote:
> From: Fabio Estevam <festevam@gmail.com> Sent: Thursday, March 31, 2016 2:37 AM
>> To: Greg Ungerer <gerg@uclinux.org>
>> Cc: Troy Kisky <troy.kisky@boundarydevices.com>; netdev@vger.kernel.org
>> Subject: Re: [PATCH] net: fec: stop the "rcv is not +last, " error messages
>>
>> Hi Greg,
>>
>> On Wed, Mar 30, 2016 at 12:24 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>>> Hi Troy,
>>>
>>> Commit 55cd48c8 ('net: fec: stop the "rcv is not +last, " error
>>> messages') adds a write to a register that is not present in all
>>> implementations of the FEC hardware module. None of the ColdFire SoC
>>> parts with the FEC module have the FTRL (0x1b0) register.
>>>
>>> Does this need a quirk flag to key access to this register of?
>>> Or can you piggyback on the FEC_QUIRK_HAS_RACC flag?
>>
>> Would the change below work on Coldfire?
>>
>> --- a/drivers/net/ethernet/freescale/fec_main.c
>> +++ b/drivers/net/ethernet/freescale/fec_main.c
>> @@ -943,8 +943,8 @@ fec_restart(struct net_device *ndev)
>> else
>> val &= ~FEC_RACC_OPTIONS;
>> writel(val, fep->hwp + FEC_RACC);
>> - }
>> writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL);
>> + }
>> #endif
>>
>> /*
>
> Fabio, we cannot do it like this that may cause confused for the quirk flag "FEC_QUIRK_HAS_RACC".
>
>
> Hi, Greg,
>
> The header file fec.h define the FEC_FTRL as below, if ColdFire SoC has no this register, we may remove the define in here and define the register according to SOC type. For example, it is ColdFire Soc, define it as 0xFFF. Is it feasible ?
>
> #if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
> defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
> ...
> #define FEC_FTRL 0x1b0
> ...
> #else
> ...
> #endif
Sure you could do that. But... you still have to be careful
with references to it in fec_main.c. They will need be conditional
on existence of FEC_FTRL, otherwise you break compilation.
Even if you define it to some bogus value we still don't
want the code actually writing to it.
Regards
Greg
^ permalink raw reply
* RE: [PATCH] net: fec: stop the "rcv is not +last, " error messages
From: Fugang Duan @ 2016-03-31 1:41 UTC (permalink / raw)
To: Fabio Estevam, Greg Ungerer; +Cc: Troy Kisky, netdev@vger.kernel.org
In-Reply-To: <CAOMZO5B79aHocuqT4jgEU9gGcFYD5ZSDZh8PrGeXYAcLHW1x5Q@mail.gmail.com>
From: Fabio Estevam <festevam@gmail.com> Sent: Thursday, March 31, 2016 2:37 AM
> To: Greg Ungerer <gerg@uclinux.org>
> Cc: Troy Kisky <troy.kisky@boundarydevices.com>; netdev@vger.kernel.org
> Subject: Re: [PATCH] net: fec: stop the "rcv is not +last, " error messages
>
> Hi Greg,
>
> On Wed, Mar 30, 2016 at 12:24 AM, Greg Ungerer <gerg@uclinux.org> wrote:
> > Hi Troy,
> >
> > Commit 55cd48c8 ('net: fec: stop the "rcv is not +last, " error
> > messages') adds a write to a register that is not present in all
> > implementations of the FEC hardware module. None of the ColdFire SoC
> > parts with the FEC module have the FTRL (0x1b0) register.
> >
> > Does this need a quirk flag to key access to this register of?
> > Or can you piggyback on the FEC_QUIRK_HAS_RACC flag?
>
> Would the change below work on Coldfire?
>
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -943,8 +943,8 @@ fec_restart(struct net_device *ndev)
> else
> val &= ~FEC_RACC_OPTIONS;
> writel(val, fep->hwp + FEC_RACC);
> - }
> writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL);
> + }
> #endif
>
> /*
Fabio, we cannot do it like this that may cause confused for the quirk flag "FEC_QUIRK_HAS_RACC".
Hi, Greg,
The header file fec.h define the FEC_FTRL as below, if ColdFire SoC has no this register, we may remove the define in here and define the register according to SOC type. For example, it is ColdFire Soc, define it as 0xFFF. Is it feasible ?
#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM)
...
#define FEC_FTRL 0x1b0
...
#else
...
#endif
Regards,
Andy
^ permalink raw reply
* RE: [PATCH 4/5] fsl/qe: Add QE TDM lib
From: Qiang Zhao @ 2016-03-31 1:25 UTC (permalink / raw)
To: Joakim Tjernlund, davem@davemloft.net
Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
Xiaobo Xie, oss@buserror.net, gregkh@linuxfoundation.org,
akpm@linux-foundation.org, netdev@vger.kernel.org
In-Reply-To: <1459338701.3538.17.camel@infinera.com>
On Wed, 2016-03-30 at 07:50PM, Joakim Tjernlund wrote:
> -----Original Message-----
> From: Joakim Tjernlund [mailto:Joakim.Tjernlund@infinera.com]
> Sent: Wednesday, March 30, 2016 7:50 PM
> To: davem@davemloft.net; Qiang Zhao <qiang.zhao@nxp.com>
> Cc: linuxppc-dev@lists.ozlabs.org; linux-kernel@vger.kernel.org; Xiaobo Xie
> <xiaobo.xie@nxp.com>; oss@buserror.net; gregkh@linuxfoundation.org;
> akpm@linux-foundation.org; netdev@vger.kernel.org
> Subject: Re: [PATCH 4/5] fsl/qe: Add QE TDM lib
>
> On Wed, 2016-03-30 at 16:50 +0800, Zhao Qiang wrote:
> > QE has module to support TDM, some other protocols supported by QE are
> > based on TDM.
> > add a qe-tdm lib, this lib provides functions to the protocols using
> > TDM to configurate QE-TDM.
> >
> > Signed-off-by: Zhao Qiang <qiang.zhao@nxp.com>
> > + utdm->siram_entry_id = val;
> > +
> > + set_si_param(utdm, ut_info);
> > +
> > + np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-si");
>
> fsl,t1040-qe-si only? What about mpc83xx?
> I recall QE is a little bit different compared to T1040 or will this work(including
> the hdlc driver) on 83xx as well?
The " fsl,t1040-qe-si " is new added to dts and bindings, it is required to have SoC specific compatible strings.
mpc83xx will not use qe-si node. If there will be other soc useing qe-si, " fsl,t1040-qe-si " will follow the soc specific compatible,
like :
si1: si@700 {
compatible = "fsl,ls1043-qe-si", "fsl,t1040-qe-si";
reg = <0x700 0x80>;
};
Best Regards
Zhao Qiang
^ permalink raw reply
* Re: [net-next] bond: output message before setting slave to inactive
From: 张胜举 @ 2016-03-31 1:23 UTC (permalink / raw)
To: 'David Miller'; +Cc: j.vosburgh, vfalico, gospo, netdev
> From: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
> Date: Tue, 29 Mar 2016 06:32:57 +0000
>
> > This patch moves output message before setting slave to inactive, this
> > will print the correct status of slave device.
> >
> > Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
>
> I think the message is in the appropriate spot wrt. state, so I will not
apply
> this, thanks.
The message will always print 'backup', since the code set it to inactive
before this output.
I think this will cause a little confused, because even when I detach an
active slave, the output is always 'backup'.
Thanks.
^ permalink raw reply
* Re: [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: Alexei Starovoitov @ 2016-03-31 1:18 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem, mkubecek, sasha.levin, jslaby, eric.dumazet, mst, netdev
In-Reply-To: <755ee9ec1f6d2229be41806964b372548e4b7586.1459382574.git.daniel@iogearbox.net>
On Thu, Mar 31, 2016 at 02:13:18AM +0200, Daniel Borkmann wrote:
> Sasha Levin reported a suspicious rcu_dereference_protected() warning
> found while fuzzing with trinity that is similar to this one:
>
> [ 52.765684] net/core/filter.c:2262 suspicious rcu_dereference_protected() usage!
> [ 52.765688] other info that might help us debug this:
> [ 52.765695] rcu_scheduler_active = 1, debug_locks = 1
> [ 52.765701] 1 lock held by a.out/1525:
> [ 52.765704] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff816a64b7>] rtnl_lock+0x17/0x20
> [ 52.765721] stack backtrace:
> [ 52.765728] CPU: 1 PID: 1525 Comm: a.out Not tainted 4.5.0+ #264
> [...]
> [ 52.765768] Call Trace:
> [ 52.765775] [<ffffffff813e488d>] dump_stack+0x85/0xc8
> [ 52.765784] [<ffffffff810f2fa5>] lockdep_rcu_suspicious+0xd5/0x110
> [ 52.765792] [<ffffffff816afdc2>] sk_detach_filter+0x82/0x90
> [ 52.765801] [<ffffffffa0883425>] tun_detach_filter+0x35/0x90 [tun]
> [ 52.765810] [<ffffffffa0884ed4>] __tun_chr_ioctl+0x354/0x1130 [tun]
> [ 52.765818] [<ffffffff8136fed0>] ? selinux_file_ioctl+0x130/0x210
> [ 52.765827] [<ffffffffa0885ce3>] tun_chr_ioctl+0x13/0x20 [tun]
> [ 52.765834] [<ffffffff81260ea6>] do_vfs_ioctl+0x96/0x690
> [ 52.765843] [<ffffffff81364af3>] ? security_file_ioctl+0x43/0x60
> [ 52.765850] [<ffffffff81261519>] SyS_ioctl+0x79/0x90
> [ 52.765858] [<ffffffff81003ba2>] do_syscall_64+0x62/0x140
> [ 52.765866] [<ffffffff817d563f>] entry_SYSCALL64_slow_path+0x25/0x25
>
> Same can be triggered with PROVE_RCU (+ PROVE_RCU_REPEATEDLY) enabled
> from tun_attach_filter() when user space calls ioctl(tun_fd, TUN{ATTACH,
> DETACH}FILTER, ...) for adding/removing a BPF filter on tap devices.
>
> Since the fix in f91ff5b9ff52 ("net: sk_{detach|attach}_filter() rcu
> fixes") sk_attach_filter()/sk_detach_filter() now dereferences the
> filter with rcu_dereference_protected(), checking whether socket lock
> is held in control path.
>
> Since its introduction in 994051625981 ("tun: socket filter support"),
> tap filters are managed under RTNL lock from __tun_chr_ioctl(). Thus the
> sock_owned_by_user(sk) doesn't apply in this specific case and therefore
> triggers the false positive.
>
> Extend the BPF API with __sk_attach_filter()/__sk_detach_filter() pair
> that is used by tap filters and pass in lockdep_rtnl_is_held() for the
> rcu_dereference_protected() checks instead.
>
> Reported-by: Sasha Levin <sasha.levin@oracle.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> drivers/net/tun.c | 8 +++++---
> include/linux/filter.h | 4 ++++
> net/core/filter.c | 33 +++++++++++++++++++++------------
> 3 files changed, 30 insertions(+), 15 deletions(-)
kinda heavy patch to shut up lockdep.
Can we do
old_fp = rcu_dereference_protected(sk->sk_filter,
sock_owned_by_user(sk) || lockdep_rtnl_is_held());
and it always be correct?
I think right now tun is the only such user, but if it's correct for tun,
it's correct for future users too. If not correct then not correct for tun either.
Or I'm missing something?
^ permalink raw reply
* Re: [PATCH] net: fec: stop the "rcv is not +last, " error messages
From: Greg Ungerer @ 2016-03-31 1:17 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Troy Kisky, netdev@vger.kernel.org
In-Reply-To: <CAOMZO5B79aHocuqT4jgEU9gGcFYD5ZSDZh8PrGeXYAcLHW1x5Q@mail.gmail.com>
Hi Fabio,
On 31/03/16 04:37, Fabio Estevam wrote:
> Hi Greg,
>
> On Wed, Mar 30, 2016 at 12:24 AM, Greg Ungerer <gerg@uclinux.org> wrote:
>> Hi Troy,
>>
>> Commit 55cd48c8 ('net: fec: stop the "rcv is not +last, " error
>> messages') adds a write to a register that is not present in all
>> implementations of the FEC hardware module. None of the ColdFire
>> SoC parts with the FEC module have the FTRL (0x1b0) register.
>>
>> Does this need a quirk flag to key access to this register of?
>> Or can you piggyback on the FEC_QUIRK_HAS_RACC flag?
>
> Would the change below work on Coldfire?
>
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -943,8 +943,8 @@ fec_restart(struct net_device *ndev)
> else
> val &= ~FEC_RACC_OPTIONS;
> writel(val, fep->hwp + FEC_RACC);
> - }
> writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL);
> + }
> #endif
>
> /*
Yes, that fixes it. Will you carry this change?
Regards
Greg
^ permalink raw reply
* Re: [PATCH net-next 0/8] add TX timestamping via cmsg
From: Soheil Hassas Yeganeh @ 2016-03-31 1:08 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: netdev
In-Reply-To: <20160331003744.GA3300@kafai-mba.local>
On Wed, Mar 30, 2016 at 8:38 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Wed, Mar 30, 2016 at 06:37:20PM -0400, Soheil Hassas Yeganeh wrote:
>> I will follow up with another patch to enable timestamping for
>> active TFO (client-side TCP Fast Open) and also setting packet
>> mark via cmsgs.
> Can you share more details on what 'setting packet mark' does?
I meant, since SOL_SOCKET-level csmg's will be processed for all
protocols, users should be able to override sk->sk_mark via command
messages when possible similar to what the following patch did for
packet sockets:
http://patchwork.ozlabs.org/patch/527991/
It'll be a tiny followup patch.
> Nice patches!
Thank you so much!
Cheers,
Soheil
^ permalink raw reply
* Re: [PATCH net-next 0/8] add TX timestamping via cmsg
From: Martin KaFai Lau @ 2016-03-31 0:38 UTC (permalink / raw)
To: Soheil Hassas Yeganeh
Cc: davem, netdev, willemb, edumazet, ycheng, ncardwell,
Soheil Hassas Yeganeh
In-Reply-To: <1459377448-2239-1-git-send-email-soheil.kdev@gmail.com>
On Wed, Mar 30, 2016 at 06:37:20PM -0400, Soheil Hassas Yeganeh wrote:
> I will follow up with another patch to enable timestamping for
> active TFO (client-side TCP Fast Open) and also setting packet
> mark via cmsgs.
Can you share more details on what 'setting packet mark' does?
Nice patches!
^ permalink raw reply
* [PATCH net] tun, bpf: fix suspicious RCU usage in tun_{attach,detach}_filter
From: Daniel Borkmann @ 2016-03-31 0:13 UTC (permalink / raw)
To: davem
Cc: alexei.starovoitov, mkubecek, sasha.levin, jslaby, eric.dumazet,
mst, netdev, Daniel Borkmann
Sasha Levin reported a suspicious rcu_dereference_protected() warning
found while fuzzing with trinity that is similar to this one:
[ 52.765684] net/core/filter.c:2262 suspicious rcu_dereference_protected() usage!
[ 52.765688] other info that might help us debug this:
[ 52.765695] rcu_scheduler_active = 1, debug_locks = 1
[ 52.765701] 1 lock held by a.out/1525:
[ 52.765704] #0: (rtnl_mutex){+.+.+.}, at: [<ffffffff816a64b7>] rtnl_lock+0x17/0x20
[ 52.765721] stack backtrace:
[ 52.765728] CPU: 1 PID: 1525 Comm: a.out Not tainted 4.5.0+ #264
[...]
[ 52.765768] Call Trace:
[ 52.765775] [<ffffffff813e488d>] dump_stack+0x85/0xc8
[ 52.765784] [<ffffffff810f2fa5>] lockdep_rcu_suspicious+0xd5/0x110
[ 52.765792] [<ffffffff816afdc2>] sk_detach_filter+0x82/0x90
[ 52.765801] [<ffffffffa0883425>] tun_detach_filter+0x35/0x90 [tun]
[ 52.765810] [<ffffffffa0884ed4>] __tun_chr_ioctl+0x354/0x1130 [tun]
[ 52.765818] [<ffffffff8136fed0>] ? selinux_file_ioctl+0x130/0x210
[ 52.765827] [<ffffffffa0885ce3>] tun_chr_ioctl+0x13/0x20 [tun]
[ 52.765834] [<ffffffff81260ea6>] do_vfs_ioctl+0x96/0x690
[ 52.765843] [<ffffffff81364af3>] ? security_file_ioctl+0x43/0x60
[ 52.765850] [<ffffffff81261519>] SyS_ioctl+0x79/0x90
[ 52.765858] [<ffffffff81003ba2>] do_syscall_64+0x62/0x140
[ 52.765866] [<ffffffff817d563f>] entry_SYSCALL64_slow_path+0x25/0x25
Same can be triggered with PROVE_RCU (+ PROVE_RCU_REPEATEDLY) enabled
from tun_attach_filter() when user space calls ioctl(tun_fd, TUN{ATTACH,
DETACH}FILTER, ...) for adding/removing a BPF filter on tap devices.
Since the fix in f91ff5b9ff52 ("net: sk_{detach|attach}_filter() rcu
fixes") sk_attach_filter()/sk_detach_filter() now dereferences the
filter with rcu_dereference_protected(), checking whether socket lock
is held in control path.
Since its introduction in 994051625981 ("tun: socket filter support"),
tap filters are managed under RTNL lock from __tun_chr_ioctl(). Thus the
sock_owned_by_user(sk) doesn't apply in this specific case and therefore
triggers the false positive.
Extend the BPF API with __sk_attach_filter()/__sk_detach_filter() pair
that is used by tap filters and pass in lockdep_rtnl_is_held() for the
rcu_dereference_protected() checks instead.
Reported-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
drivers/net/tun.c | 8 +++++---
include/linux/filter.h | 4 ++++
net/core/filter.c | 33 +++++++++++++++++++++------------
3 files changed, 30 insertions(+), 15 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index afdf950..510e90a 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -622,7 +622,8 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
/* Re-attach the filter to persist device */
if (!skip_filter && (tun->filter_attached == true)) {
- err = sk_attach_filter(&tun->fprog, tfile->socket.sk);
+ err = __sk_attach_filter(&tun->fprog, tfile->socket.sk,
+ lockdep_rtnl_is_held());
if (!err)
goto out;
}
@@ -1822,7 +1823,7 @@ static void tun_detach_filter(struct tun_struct *tun, int n)
for (i = 0; i < n; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
- sk_detach_filter(tfile->socket.sk);
+ __sk_detach_filter(tfile->socket.sk, lockdep_rtnl_is_held());
}
tun->filter_attached = false;
@@ -1835,7 +1836,8 @@ static int tun_attach_filter(struct tun_struct *tun)
for (i = 0; i < tun->numqueues; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
- ret = sk_attach_filter(&tun->fprog, tfile->socket.sk);
+ ret = __sk_attach_filter(&tun->fprog, tfile->socket.sk,
+ lockdep_rtnl_is_held());
if (ret) {
tun_detach_filter(tun, i);
return ret;
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 43aa1f8..a51a536 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -465,10 +465,14 @@ int bpf_prog_create_from_user(struct bpf_prog **pfp, struct sock_fprog *fprog,
void bpf_prog_destroy(struct bpf_prog *fp);
int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
+int __sk_attach_filter(struct sock_fprog *fprog, struct sock *sk,
+ bool locked);
int sk_attach_bpf(u32 ufd, struct sock *sk);
int sk_reuseport_attach_filter(struct sock_fprog *fprog, struct sock *sk);
int sk_reuseport_attach_bpf(u32 ufd, struct sock *sk);
int sk_detach_filter(struct sock *sk);
+int __sk_detach_filter(struct sock *sk, bool locked);
+
int sk_get_filter(struct sock *sk, struct sock_filter __user *filter,
unsigned int len);
diff --git a/net/core/filter.c b/net/core/filter.c
index 4b81b71..ca7f832 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1149,7 +1149,8 @@ void bpf_prog_destroy(struct bpf_prog *fp)
}
EXPORT_SYMBOL_GPL(bpf_prog_destroy);
-static int __sk_attach_prog(struct bpf_prog *prog, struct sock *sk)
+static int __sk_attach_prog(struct bpf_prog *prog, struct sock *sk,
+ bool locked)
{
struct sk_filter *fp, *old_fp;
@@ -1165,10 +1166,8 @@ static int __sk_attach_prog(struct bpf_prog *prog, struct sock *sk)
return -ENOMEM;
}
- old_fp = rcu_dereference_protected(sk->sk_filter,
- sock_owned_by_user(sk));
+ old_fp = rcu_dereference_protected(sk->sk_filter, locked);
rcu_assign_pointer(sk->sk_filter, fp);
-
if (old_fp)
sk_filter_uncharge(sk, old_fp);
@@ -1247,7 +1246,8 @@ struct bpf_prog *__get_filter(struct sock_fprog *fprog, struct sock *sk)
* occurs or there is insufficient memory for the filter a negative
* errno code is returned. On success the return is zero.
*/
-int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
+int __sk_attach_filter(struct sock_fprog *fprog, struct sock *sk,
+ bool locked)
{
struct bpf_prog *prog = __get_filter(fprog, sk);
int err;
@@ -1255,7 +1255,7 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
if (IS_ERR(prog))
return PTR_ERR(prog);
- err = __sk_attach_prog(prog, sk);
+ err = __sk_attach_prog(prog, sk, locked);
if (err < 0) {
__bpf_prog_release(prog);
return err;
@@ -1263,7 +1263,12 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
return 0;
}
-EXPORT_SYMBOL_GPL(sk_attach_filter);
+EXPORT_SYMBOL_GPL(__sk_attach_filter);
+
+int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
+{
+ return __sk_attach_filter(fprog, sk, sock_owned_by_user(sk));
+}
int sk_reuseport_attach_filter(struct sock_fprog *fprog, struct sock *sk)
{
@@ -1309,7 +1314,7 @@ int sk_attach_bpf(u32 ufd, struct sock *sk)
if (IS_ERR(prog))
return PTR_ERR(prog);
- err = __sk_attach_prog(prog, sk);
+ err = __sk_attach_prog(prog, sk, sock_owned_by_user(sk));
if (err < 0) {
bpf_prog_put(prog);
return err;
@@ -2250,7 +2255,7 @@ static int __init register_sk_filter_ops(void)
}
late_initcall(register_sk_filter_ops);
-int sk_detach_filter(struct sock *sk)
+int __sk_detach_filter(struct sock *sk, bool locked)
{
int ret = -ENOENT;
struct sk_filter *filter;
@@ -2258,8 +2263,7 @@ int sk_detach_filter(struct sock *sk)
if (sock_flag(sk, SOCK_FILTER_LOCKED))
return -EPERM;
- filter = rcu_dereference_protected(sk->sk_filter,
- sock_owned_by_user(sk));
+ filter = rcu_dereference_protected(sk->sk_filter, locked);
if (filter) {
RCU_INIT_POINTER(sk->sk_filter, NULL);
sk_filter_uncharge(sk, filter);
@@ -2268,7 +2272,12 @@ int sk_detach_filter(struct sock *sk)
return ret;
}
-EXPORT_SYMBOL_GPL(sk_detach_filter);
+EXPORT_SYMBOL_GPL(__sk_detach_filter);
+
+int sk_detach_filter(struct sock *sk)
+{
+ return __sk_detach_filter(sk, sock_owned_by_user(sk));
+}
int sk_get_filter(struct sock *sk, struct sock_filter __user *ubuf,
unsigned int len)
--
1.9.3
^ permalink raw reply related
* Re: [PATCH iproute2 v1 1/1] lib/utils: fix get_addr() and get_prefix() error messages
From: Stephen Hemminger @ 2016-03-30 23:54 UTC (permalink / raw)
To: Varlese, Marco
Cc: netdev@vger.kernel.org, davem@davemloft.net, Jiri Pirko,
John Fastabend, jhs@mojatatu.com, Szczerbik, PrzemyslawX
In-Reply-To: <C4896FB061E7DE4AAC93031BDCA044B11A2146FF@IRSMSX108.ger.corp.intel.com>
On Tue, 22 Mar 2016 13:02:02 +0000
"Varlese, Marco" <marco.varlese@intel.com> wrote:
> An attempt to add invalid address to interface would print "???" string
> instead of the address family name.
>
> For example:
> $ ip address add 256.10.166.1/24 dev ens8
> Error: ??? prefix is expected rather than "256.10.166.1/24".
>
> $ ip neighbor add proxy 2001:db8::g dev ens8
> Error: ??? address is expected rather than "2001:db8::g".
>
> With this patch the output will look like:
> $ ip address add 256.10.166.1/24 dev ens8
> Error: inet prefix is expected rather than "256.10.166.1/24".
>
> $ ip neighbor add proxy 2001:db8::g dev ens8
> Error: inet6 address is expected rather than "2001:db8::g".
>
> Signed-off-by: Przemyslaw Szczerbik <przemyslawx.szczerbik@intel.com>
> Signed-off-by: Marco Varlese <marco.varlese@intel.com>
> ---
> lib/utils.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
If you look at git, I already applied this by manual fix.
^ permalink raw reply
* Re: [net PATCH v2] i40e/i40evf: Limit TSO to 7 descriptors for payload instead of 8 per packet
From: Sowmini Varadhan @ 2016-03-30 23:51 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, jesse.brandeburg, alexander.duyck, intel-wired-lan,
jeffrey.t.kirsher
In-Reply-To: <20160330231116.12643.59554.stgit@localhost.localdomain>
Yes, this fixes it for me too!
Tested-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
^ permalink raw reply
* [net PATCH v2] i40e/i40evf: Limit TSO to 7 descriptors for payload instead of 8 per packet
From: Alexander Duyck @ 2016-03-30 23:15 UTC (permalink / raw)
To: netdev, jesse.brandeburg, alexander.duyck, intel-wired-lan,
jeffrey.t.kirsher, sowmini.varadhan
This patch addresses a bug introduced based on my interpretation of the
XL710 datasheet. Specifically section 8.4.1 states that "A single transmit
packet may span up to 8 buffers (up to 8 data descriptors per packet
including both the header and payload buffers)." It then later goes on to
say that each segment for a TSO obeys the previous rule, however it then
refers to TSO header and the segment payload buffers.
I believe the actual limit for fragments with TSO and a skbuff that has
payload data in the header portion of the buffer is actually only 7
fragments as the skb->data portion counts as 2 buffers, one for the TSO
header, and one for a segment payload buffer.
Fixes: 2d37490b82af ("i40e/i40evf: Rewrite logic for 8 descriptor per packet check")
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
v2: I realized that I overlooked the check in the inline function and as a
result we were still allowing for cases where 8 descriptors were being
used per packet and this would result in 9 DMA buffers. I updated the
code so that we only allow 8 in the case of a single send, otherwise we
go into the function that walks the frags to verify each block.
I have tested this using rds-stress and it seems to run traffic without
throwing any errors.
drivers/net/ethernet/intel/i40e/i40e_txrx.c | 49 ++++++++++++-------------
drivers/net/ethernet/intel/i40e/i40e_txrx.h | 10 ++++-
drivers/net/ethernet/intel/i40evf/i40e_txrx.c | 49 ++++++++++++-------------
drivers/net/ethernet/intel/i40evf/i40e_txrx.h | 10 ++++-
4 files changed, 62 insertions(+), 56 deletions(-)
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
index 5d5fa5359a1d..6bf705add321 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c
@@ -2597,35 +2597,34 @@ int __i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size)
}
/**
- * __i40e_chk_linearize - Check if there are more than 8 fragments per packet
+ * __i40e_chk_linearize - Check if there are more than 8 buffers per packet
* @skb: send buffer
*
- * Note: Our HW can't scatter-gather more than 8 fragments to build
- * a packet on the wire and so we need to figure out the cases where we
- * need to linearize the skb.
+ * Note: Our HW can't DMA more than 8 buffers to build a packet on the wire
+ * and so we need to figure out the cases where we need to linearize the skb.
+ *
+ * For TSO we need to count the TSO header and segment payload separately.
+ * As such we need to check cases where we have 7 fragments or more as we
+ * can potentially require 9 DMA transactions, 1 for the TSO header, 1 for
+ * the segment payload in the first descriptor, and another 7 for the
+ * fragments.
**/
bool __i40e_chk_linearize(struct sk_buff *skb)
{
const struct skb_frag_struct *frag, *stale;
- int gso_size, nr_frags, sum;
-
- /* check to see if TSO is enabled, if so we may get a repreive */
- gso_size = skb_shinfo(skb)->gso_size;
- if (unlikely(!gso_size))
- return true;
+ int nr_frags, sum;
- /* no need to check if number of frags is less than 8 */
+ /* no need to check if number of frags is less than 7 */
nr_frags = skb_shinfo(skb)->nr_frags;
- if (nr_frags < I40E_MAX_BUFFER_TXD)
+ if (nr_frags < (I40E_MAX_BUFFER_TXD - 1))
return false;
/* We need to walk through the list and validate that each group
* of 6 fragments totals at least gso_size. However we don't need
- * to perform such validation on the first or last 6 since the first
- * 6 cannot inherit any data from a descriptor before them, and the
- * last 6 cannot inherit any data from a descriptor after them.
+ * to perform such validation on the last 6 since the last 6 cannot
+ * inherit any data from a descriptor after them.
*/
- nr_frags -= I40E_MAX_BUFFER_TXD - 1;
+ nr_frags -= I40E_MAX_BUFFER_TXD - 2;
frag = &skb_shinfo(skb)->frags[0];
/* Initialize size to the negative value of gso_size minus 1. We
@@ -2634,21 +2633,21 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
* descriptors for a single transmit as the header and previous
* fragment are already consuming 2 descriptors.
*/
- sum = 1 - gso_size;
+ sum = 1 - skb_shinfo(skb)->gso_size;
- /* Add size of frags 1 through 5 to create our initial sum */
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
+ /* Add size of frags 0 through 4 to create our initial sum */
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
/* Walk through fragments adding latest fragment, testing it, and
* then removing stale fragments from the sum.
*/
stale = &skb_shinfo(skb)->frags[0];
for (;;) {
- sum += skb_frag_size(++frag);
+ sum += skb_frag_size(frag++);
/* if sum is negative we failed to make sufficient progress */
if (sum < 0)
@@ -2658,7 +2657,7 @@ bool __i40e_chk_linearize(struct sk_buff *skb)
if (!--nr_frags)
break;
- sum -= skb_frag_size(++stale);
+ sum -= skb_frag_size(stale++);
}
return false;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
index 681e9bca37db..c864932bd844 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
@@ -442,10 +442,14 @@ static inline int i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size)
**/
static inline bool i40e_chk_linearize(struct sk_buff *skb, int count)
{
- /* we can only support up to 8 data buffers for a single send */
- if (likely(count <= I40E_MAX_BUFFER_TXD))
+ /* Both TSO and single send will work if count is less than 8 */
+ if (likely(count < I40E_MAX_BUFFER_TXD))
return false;
- return __i40e_chk_linearize(skb);
+ if (skb_is_gso(skb))
+ return __i40e_chk_linearize(skb);
+
+ /* we can support up to 8 data buffers for a single send */
+ return count != I40E_MAX_BUFFER_TXD;
}
#endif /* _I40E_TXRX_H_ */
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
index 04aabc52ba0d..519256bb63e8 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.c
@@ -1799,35 +1799,34 @@ static void i40e_create_tx_ctx(struct i40e_ring *tx_ring,
}
/**
- * __i40evf_chk_linearize - Check if there are more than 8 fragments per packet
+ * __i40evf_chk_linearize - Check if there are more than 8 buffers per packet
* @skb: send buffer
*
- * Note: Our HW can't scatter-gather more than 8 fragments to build
- * a packet on the wire and so we need to figure out the cases where we
- * need to linearize the skb.
+ * Note: Our HW can't DMA more than 8 buffers to build a packet on the wire
+ * and so we need to figure out the cases where we need to linearize the skb.
+ *
+ * For TSO we need to count the TSO header and segment payload separately.
+ * As such we need to check cases where we have 7 fragments or more as we
+ * can potentially require 9 DMA transactions, 1 for the TSO header, 1 for
+ * the segment payload in the first descriptor, and another 7 for the
+ * fragments.
**/
bool __i40evf_chk_linearize(struct sk_buff *skb)
{
const struct skb_frag_struct *frag, *stale;
- int gso_size, nr_frags, sum;
-
- /* check to see if TSO is enabled, if so we may get a repreive */
- gso_size = skb_shinfo(skb)->gso_size;
- if (unlikely(!gso_size))
- return true;
+ int nr_frags, sum;
- /* no need to check if number of frags is less than 8 */
+ /* no need to check if number of frags is less than 7 */
nr_frags = skb_shinfo(skb)->nr_frags;
- if (nr_frags < I40E_MAX_BUFFER_TXD)
+ if (nr_frags < (I40E_MAX_BUFFER_TXD - 1))
return false;
/* We need to walk through the list and validate that each group
* of 6 fragments totals at least gso_size. However we don't need
- * to perform such validation on the first or last 6 since the first
- * 6 cannot inherit any data from a descriptor before them, and the
- * last 6 cannot inherit any data from a descriptor after them.
+ * to perform such validation on the last 6 since the last 6 cannot
+ * inherit any data from a descriptor after them.
*/
- nr_frags -= I40E_MAX_BUFFER_TXD - 1;
+ nr_frags -= I40E_MAX_BUFFER_TXD - 2;
frag = &skb_shinfo(skb)->frags[0];
/* Initialize size to the negative value of gso_size minus 1. We
@@ -1836,21 +1835,21 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
* descriptors for a single transmit as the header and previous
* fragment are already consuming 2 descriptors.
*/
- sum = 1 - gso_size;
+ sum = 1 - skb_shinfo(skb)->gso_size;
- /* Add size of frags 1 through 5 to create our initial sum */
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
- sum += skb_frag_size(++frag);
+ /* Add size of frags 0 through 4 to create our initial sum */
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
+ sum += skb_frag_size(frag++);
/* Walk through fragments adding latest fragment, testing it, and
* then removing stale fragments from the sum.
*/
stale = &skb_shinfo(skb)->frags[0];
for (;;) {
- sum += skb_frag_size(++frag);
+ sum += skb_frag_size(frag++);
/* if sum is negative we failed to make sufficient progress */
if (sum < 0)
@@ -1860,7 +1859,7 @@ bool __i40evf_chk_linearize(struct sk_buff *skb)
if (!--nr_frags)
break;
- sum -= skb_frag_size(++stale);
+ sum -= skb_frag_size(stale++);
}
return false;
diff --git a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
index 6cf116972f62..396c3893ce8a 100644
--- a/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
+++ b/drivers/net/ethernet/intel/i40evf/i40e_txrx.h
@@ -424,10 +424,14 @@ static inline int i40e_maybe_stop_tx(struct i40e_ring *tx_ring, int size)
**/
static inline bool i40e_chk_linearize(struct sk_buff *skb, int count)
{
- /* we can only support up to 8 data buffers for a single send */
- if (likely(count <= I40E_MAX_BUFFER_TXD))
+ /* Both TSO and single send will work if count is less than 8 */
+ if (likely(count < I40E_MAX_BUFFER_TXD))
return false;
- return __i40evf_chk_linearize(skb);
+ if (skb_is_gso(skb))
+ return __i40evf_chk_linearize(skb);
+
+ /* we can support up to 8 data buffers for a single send */
+ return count != I40E_MAX_BUFFER_TXD;
}
#endif /* _I40E_TXRX_H_ */
^ permalink raw reply related
* Re: [net PATCH] i40e/i40evf: Limit TSO to 7 descriptors for payload instead of 8 per packet
From: Alexander Duyck @ 2016-03-30 23:06 UTC (permalink / raw)
To: Jesse Brandeburg
Cc: Sowmini Varadhan, Alexander Duyck, Netdev, intel-wired-lan,
Jeff Kirsher
In-Reply-To: <CAKgT0Ud59j1NDHD6siPSZL24HJnPOF7m07DKf4U96Tq4dtByvg@mail.gmail.com>
On Wed, Mar 30, 2016 at 2:20 PM, Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Wed, Mar 30, 2016 at 12:41 PM, Jesse Brandeburg
> <jesse.brandeburg@intel.com> wrote:
>> On Wed, 30 Mar 2016 10:35:55 -0700
>> Alexander Duyck <alexander.duyck@gmail.com> wrote:
>>
>>> On Wed, Mar 30, 2016 at 10:20 AM, Sowmini Varadhan
>>> <sowmini.varadhan@oracle.com> wrote:
>>> > On (03/30/16 10:12), Alexander Duyck wrote:
>>> >> Yeah. The patch was sort of a knee-jerk reaction to being told that
>>> >> the patch referenced caused a regression. From what I can tell that
>>> >> is not the case as I am also seeing the Tx hangs when I run the test
>>> >> with the frames being linearized.
>>> >
>>> > I'm not sure how important of a subtlety this is, but the actual
>>> > console log after the patch is the following:
>>> >
>>> > i40e 0000:82:00.0: TX driver issue detected, PF reset issued
>>> > i40e 0000:82:00.0 eth2: adding 68:05:ca:30:dd:18 vid=0
>>> > i40e 0000:82:00.0: TX driver issue detected, PF reset issued
>>> > i40e 0000:82:00.0 eth2: adding 68:05:ca:30:dd:18 vid=0
>>> > i40e 0000:82:00.0: TX driver issue detected, PF reset issued
>>> >
>>> > Comparing with what I'd pasted in the sourceforge thread earlier,
>>> > I see that it does not say "Hung Tx queue etc." any more, though
>>> > it still resets.
>>> >
>>> > Not sure if that changed info is significant?
>>>
>>> It might be. Right now I am chasing down the Tx driver issue as that
>>> I what I am reproducing in my environment as well.
>>
>> This gets "Even Uglier", I've turned off all offloads at my receiver,
>> enabled calling skb_linearize on *all* frames, which works fine for
>> scp, but the receiver shows > MSS sized frames on the wire for
>> rds-stress traffic.
>
> Are you sure it isn't just GRO reassembling frames on the receive
> side. I know that one always trips me up when I am using the Rx path
> to validate Tx checksums.
>
>> This implies to me we have some issue with skb_linearize, possibly in
>> how the stack linearizes the data, or how the driver interprets the
>> linearized packets (which should always work)
>>
>> Wheee......
>
> With the descriptor dump code you have you should be able to verify
> what the layout is after the descriptor is linearized. I would think
> in most cases you would end up with at most something like 4 to maybe
> 5 descriptors for a 64K frame.
>
> - Alex
Actually I think I just found an issue I missed in the patch. I
didn't update the inline function that was performing the check for 8
descriptors. As such it was allowing TSO with 8 descriptors to pass
even though the fact that the head and payload in the first descriptor
had pushed it to 9.
I should have a v2 ready in about 20 minutes or so. In my testing it
fixes the issue.
- Alex
^ permalink raw reply
* [net 07/11] ixgbevf: fix error code path when setting MAC address
From: Jeff Kirsher @ 2016-03-30 23:00 UTC (permalink / raw)
To: davem; +Cc: Emil Tantilov, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1459378855-139837-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Emil Tantilov <emil.s.tantilov@intel.com>
Return error when a MAC address change is rejected by the PF.
This will prevent the user from modifying the MAC address when
that operation is not permitted.
Signed-off-by: Emil Tantilov <emil.s.tantilov@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 12 ++++++++----
drivers/net/ethernet/intel/ixgbevf/vf.c | 4 +++-
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 6a337bb..b0edae9 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3692,19 +3692,23 @@ static int ixgbevf_set_mac(struct net_device *netdev, void *p)
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
struct sockaddr *addr = p;
+ int err;
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
- ether_addr_copy(netdev->dev_addr, addr->sa_data);
- ether_addr_copy(hw->mac.addr, addr->sa_data);
-
spin_lock_bh(&adapter->mbx_lock);
- hw->mac.ops.set_rar(hw, 0, hw->mac.addr, 0);
+ err = hw->mac.ops.set_rar(hw, 0, addr->sa_data, 0);
spin_unlock_bh(&adapter->mbx_lock);
+ if (err)
+ return -EPERM;
+
+ ether_addr_copy(hw->mac.addr, addr->sa_data);
+ ether_addr_copy(netdev->dev_addr, addr->sa_data);
+
return 0;
}
diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
index 61a98f4..4d613a4 100644
--- a/drivers/net/ethernet/intel/ixgbevf/vf.c
+++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
@@ -408,8 +408,10 @@ static s32 ixgbevf_set_rar_vf(struct ixgbe_hw *hw, u32 index, u8 *addr,
/* if nacked the address was rejected, use "perm_addr" */
if (!ret_val &&
- (msgbuf[0] == (IXGBE_VF_SET_MAC_ADDR | IXGBE_VT_MSGTYPE_NACK)))
+ (msgbuf[0] == (IXGBE_VF_SET_MAC_ADDR | IXGBE_VT_MSGTYPE_NACK))) {
ixgbevf_get_mac_addr_vf(hw, hw->mac.addr);
+ return IXGBE_ERR_MBX;
+ }
return ret_val;
}
--
2.5.5
^ permalink raw reply related
* [net 05/11] ixgbe: call ndo_stop() instead of dev_close() when running offline selftest
From: Jeff Kirsher @ 2016-03-30 23:00 UTC (permalink / raw)
To: davem; +Cc: Stefan Assmann, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1459378855-139837-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Stefan Assmann <sassmann@kpanic.de>
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 458549c..e4949af 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -877,6 +877,8 @@ extern const char ixgbe_driver_version[];
extern char ixgbe_default_device_descr[];
#endif /* IXGBE_FCOE */
+int ixgbe_open(struct net_device *netdev);
+int ixgbe_close(struct net_device *netdev);
void ixgbe_up(struct ixgbe_adapter *adapter);
void ixgbe_down(struct ixgbe_adapter *adapter);
void ixgbe_reinit_locked(struct ixgbe_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 726e0ee..b3530e1 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -2053,7 +2053,7 @@ static void ixgbe_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbe_close(netdev);
else
ixgbe_reset(adapter);
@@ -2091,7 +2091,7 @@ skip_loopback:
/* clear testing bit and return adapter to previous state */
clear_bit(__IXGBE_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbe_open(netdev);
else if (hw->mac.ops.disable_tx_laser)
hw->mac.ops.disable_tx_laser(hw);
} else {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index ee81618..267a507 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5992,7 +5992,7 @@ static int ixgbe_change_mtu(struct net_device *netdev, int new_mtu)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbe_open(struct net_device *netdev)
+int ixgbe_open(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -6094,7 +6094,7 @@ static void ixgbe_close_suspend(struct ixgbe_adapter *adapter)
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbe_close(struct net_device *netdev)
+int ixgbe_close(struct net_device *netdev)
{
struct ixgbe_adapter *adapter = netdev_priv(netdev);
--
2.5.5
^ permalink raw reply related
* [net 06/11] ixgbevf: call ndo_stop() instead of dev_close() when running offline selftest
From: Jeff Kirsher @ 2016-03-30 23:00 UTC (permalink / raw)
To: davem; +Cc: Stefan Assmann, netdev, nhorman, sassmann, jogreene, Jeff Kirsher
In-Reply-To: <1459378855-139837-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Stefan Assmann <sassmann@kpanic.de>
Calling dev_close() causes IFF_UP to be cleared which will remove the
interfaces routes and some addresses. That's probably not what the user
intended when running the offline selftest. Besides this does not happen
if the interface is brought down before the test, so the current
behaviour is inconsistent.
Instead call the net_device_ops ndo_stop function directly and avoid
touching IFF_UP at all.
Signed-off-by: Stefan Assmann <sassmann@kpanic.de>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 4 ++--
drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 2 ++
drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 ++--
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index c48aef6..d7aa4b2 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -680,7 +680,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
if (if_running)
/* indicate we're in test mode */
- dev_close(netdev);
+ ixgbevf_close(netdev);
else
ixgbevf_reset(adapter);
@@ -692,7 +692,7 @@ static void ixgbevf_diag_test(struct net_device *netdev,
clear_bit(__IXGBEVF_TESTING, &adapter->state);
if (if_running)
- dev_open(netdev);
+ ixgbevf_open(netdev);
} else {
hw_dbg(&adapter->hw, "online testing starting\n");
/* Online tests */
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
index 68ec7daa..991eeae 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h
@@ -486,6 +486,8 @@ extern const struct ixgbe_mbx_operations ixgbevf_mbx_ops;
extern const char ixgbevf_driver_name[];
extern const char ixgbevf_driver_version[];
+int ixgbevf_open(struct net_device *netdev);
+int ixgbevf_close(struct net_device *netdev);
void ixgbevf_up(struct ixgbevf_adapter *adapter);
void ixgbevf_down(struct ixgbevf_adapter *adapter);
void ixgbevf_reinit_locked(struct ixgbevf_adapter *adapter);
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 0ea14c0..6a337bb 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3122,7 +3122,7 @@ static void ixgbevf_free_all_rx_resources(struct ixgbevf_adapter *adapter)
* handler is registered with the OS, the watchdog timer is started,
* and the stack is notified that the interface is ready.
**/
-static int ixgbevf_open(struct net_device *netdev)
+int ixgbevf_open(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
struct ixgbe_hw *hw = &adapter->hw;
@@ -3205,7 +3205,7 @@ err_setup_reset:
* needs to be disabled. A global MAC reset is issued to stop the
* hardware, and all transmit and receive resources are freed.
**/
-static int ixgbevf_close(struct net_device *netdev)
+int ixgbevf_close(struct net_device *netdev)
{
struct ixgbevf_adapter *adapter = netdev_priv(netdev);
--
2.5.5
^ permalink raw reply related
* [net 03/11] ixgbe: Fix ATR so that it correctly handles IPv6 extension headers
From: Jeff Kirsher @ 2016-03-30 23:00 UTC (permalink / raw)
To: davem; +Cc: Alexander Duyck, netdev, nhorman, sassmann, jogreene,
Jeff Kirsher
In-Reply-To: <1459378855-139837-1-git-send-email-jeffrey.t.kirsher@intel.com>
From: Alexander Duyck <aduyck@mirantis.com>
The ATR code was assuming that it would be able to use tcp_hdr for
every TCP frame that came through. However this isn't the case as it
is possible for a frame to arrive that is TCP but sent through something
like a raw socket. As a result the driver was setting up bad filters in
which tcp_hdr was really pointing to the network header so the data was
all invalid.
In order to correct this I have added a bit of parsing logic that will
determine the TCP header location based off of the network header and
either the offset in the case of the IPv4 header, or a walk through the
IPv6 extension headers until it encounters the header that indicates
IPPROTO_TCP. In addition I have added checks to verify that the lowest
protocol provided is recognized as IPv4 or IPv6 to help mitigate raw
sockets using ETH_P_ALL from having ATR applied to them.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 45 +++++++++++++--------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f67c9a6..ee81618 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7558,8 +7558,10 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
struct ipv6hdr *ipv6;
} hdr;
struct tcphdr *th;
+ unsigned int hlen;
struct sk_buff *skb;
__be16 vlan_id;
+ int l4_proto;
/* if ring doesn't have a interrupt vector, cannot perform ATR */
if (!q_vector)
@@ -7571,10 +7573,14 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
ring->atr_count++;
+ /* currently only IPv4/IPv6 with TCP is supported */
+ if ((first->protocol != htons(ETH_P_IP)) &&
+ (first->protocol != htons(ETH_P_IPV6)))
+ return;
+
/* snag network header to get L4 type and address */
skb = first->skb;
hdr.network = skb_network_header(skb);
- th = tcp_hdr(skb);
#ifdef CONFIG_IXGBE_VXLAN
if (skb->encapsulation &&
first->protocol == htons(ETH_P_IP) &&
@@ -7583,43 +7589,34 @@ static void ixgbe_atr(struct ixgbe_ring *ring,
/* verify the port is recognized as VXLAN */
if (adapter->vxlan_port &&
- udp_hdr(skb)->dest == adapter->vxlan_port) {
+ udp_hdr(skb)->dest == adapter->vxlan_port)
hdr.network = skb_inner_network_header(skb);
- th = inner_tcp_hdr(skb);
- }
}
#endif /* CONFIG_IXGBE_VXLAN */
/* Currently only IPv4/IPv6 with TCP is supported */
switch (hdr.ipv4->version) {
case IPVERSION:
- if (hdr.ipv4->protocol != IPPROTO_TCP)
- return;
+ /* access ihl as u8 to avoid unaligned access on ia64 */
+ hlen = (hdr.network[0] & 0x0F) << 2;
+ l4_proto = hdr.ipv4->protocol;
break;
case 6:
- if (likely((unsigned char *)th - hdr.network ==
- sizeof(struct ipv6hdr))) {
- if (hdr.ipv6->nexthdr != IPPROTO_TCP)
- return;
- } else {
- __be16 frag_off;
- u8 l4_hdr;
-
- ipv6_skip_exthdr(skb, hdr.network - skb->data +
- sizeof(struct ipv6hdr),
- &l4_hdr, &frag_off);
- if (unlikely(frag_off))
- return;
- if (l4_hdr != IPPROTO_TCP)
- return;
- }
+ hlen = hdr.network - skb->data;
+ l4_proto = ipv6_find_hdr(skb, &hlen, IPPROTO_TCP, NULL, NULL);
+ hlen -= hdr.network - skb->data;
break;
default:
return;
}
- /* skip this packet since it is invalid or the socket is closing */
- if (!th || th->fin)
+ if (l4_proto != IPPROTO_TCP)
+ return;
+
+ th = (struct tcphdr *)(hdr.network + hlen);
+
+ /* skip this packet since the socket is closing */
+ if (th->fin)
return;
/* sample on all syn packets or once every atr sample count */
--
2.5.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox