* Re: [RFC] Add BPF_SYNCHRONIZE bpf(2) command
From: Joel Fernandes @ 2018-07-30 22:26 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Daniel Colascione, Lorenzo Colitti, Chenbo Feng,
Mathieu Desnoyers, Joel Fernandes, Alexei Starovoitov, lkml,
Tim Murray, Daniel Borkmann, netdev
In-Reply-To: <CAADnVQL=yTRvNLun2_oiwDvUvpvTV9=pOLyNhu=_cQ2sX04p5w@mail.gmail.com>
On Sun, Jul 29, 2018 at 06:57:06PM +0300, Alexei Starovoitov wrote:
> On Fri, Jul 27, 2018 at 10:17 PM, Daniel Colascione <dancol@google.com> wrote:
> > On Sat, Jul 14, 2018 at 11:18 AM, Joel Fernandes <joel@joelfernandes.org>
> > wrote:
> >>
> >> By the way just curious I was briefly going through kernel/bpf/arraymap.c.
> >> How are you protecting against load-store tearing of values of array map
> >> updates/lookups?
> >>
> >> For example, if userspace reads an array map at a particular index, while
> >> another CPU is updating it, then userspace can read partial values /
> >> half-updated values right? Since rcu_read_lock is in use, I was hoping to
> >> find something like rcu_assign_pointer there to protect readers against
> >> concurrent updates. Thanks for any clarification.
> >
> >
> > I'm also curious about the answer to this question.
>
> i'm not sure I understand the question.
> bpf_map_type_array is a C-like array.
> There is no locking of elements.
> If single program executing on two cpus
> and accesses the same value it will collide.
> Same goes for user space vs prog parallel access.
> bpf long_memcpy is an attempt to provide minimal
> level of automicity when values are aligned and
> size==long.
Thanks for the answer. I think you answered the question.
Regards,
- Joel
^ permalink raw reply
* Re: [PATCH v2 1/7] dt-bindings: net: Fix Broadcom iProc mdio mux driver base address
From: Florian Fainelli @ 2018-07-30 22:48 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-2-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Modify the base address of the Broadcom iProc MDIO mux driver to
> point to the start of the block's register address space.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2 2/7] net: phy: Fix the register offsets in Broadcom iProc mdio mux driver
From: Florian Fainelli @ 2018-07-30 22:50 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-3-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Modify the register offsets in the Broadcom iProc mdio mux to start
> from the top of the register address space.
>
> Earlier, the base address pointed to the end of the block's register
> space. The base address will now point to the start of the mdio's
> address space. The offsets have been fixed to match this.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2 4/7] net: phy: Disable external master access in bcm mdio mux driver
From: Florian Fainelli @ 2018-07-30 22:51 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-5-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Configure the scan control register in the Broadcom iProc
> mdio mux driver to disable access to external master.
>
> In some SoC's, the scan control register defaults to an incorrect
> value.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2 5/7] dt-bindings: net: Add clock handle to Broadcom iProc mdio mux
From: Florian Fainelli @ 2018-07-30 22:51 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-6-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Add clock phandle, of the core clock driving the mdio block, as an
> optional property to the Broadcom iProc mdio mux.
>
> The clock, when specified, will be used to setup the rate adjust registers
> in the mdio to derrive the mdio's operating frequency.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH v2 7/7] net: phy: Add pm support to Broadcom iProc mdio mux driver
From: Florian Fainelli @ 2018-07-30 22:53 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-8-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Add support for suspend and resume to the Broadcom iProc mdio
> mux driver.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
> ---
> drivers/net/phy/mdio-mux-bcm-iproc.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
>
> diff --git a/drivers/net/phy/mdio-mux-bcm-iproc.c b/drivers/net/phy/mdio-mux-bcm-iproc.c
> index 51d1003..d89dce0 100644
> --- a/drivers/net/phy/mdio-mux-bcm-iproc.c
> +++ b/drivers/net/phy/mdio-mux-bcm-iproc.c
> @@ -290,6 +290,33 @@ static int mdio_mux_iproc_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int mdio_mux_iproc_suspend(struct device *dev)
> +{
> + struct iproc_mdiomux_desc *md = dev_get_drvdata(dev);
> +
> + if (md->core_clk)
> + clk_disable_unprepare(md->core_clk);
Since you are assigning md->core_clk to NULL in case clk_get() returned
an error, checking for md->core_clk is not necessary since the Linux
common clock framework perform that NULL check already.
--
--
Florian
^ permalink raw reply
* Re: Security enhancement proposal for kernel TLS
From: Dave Watson @ 2018-07-30 21:16 UTC (permalink / raw)
To: Vakul Garg; +Cc: netdev@vger.kernel.org, Peter Doliwa, Boris Pismenny
In-Reply-To: <DB7PR04MB425254659CB45255A39A2A448B2F0@DB7PR04MB4252.eurprd04.prod.outlook.com>
On 07/30/18 06:31 AM, Vakul Garg wrote:
> > It's not entirely clear how your TLS handshake daemon works - Why is
> > it necessary to set the keys in the kernel tls socket before the handshake is
> > completed?
>
> IIUC, with the upstream implementation of tls record layer in kernel, the
> decryption of tls FINISHED message happens in kernel. Therefore the keys are
> already being sent to kernel tls socket before handshake is completed.
This is incorrect. Currently the kernel TLS implementation decrypts
everything after you set the keys on the socket. I'm suggesting that
you don't set the keys on the socket until after the FINISHED message.
> > Or, why do you need to hand off the fd to the client program
> > before the handshake is completed?
>
> The fd is always owned by the client program..
>
> In my proposal, the applications poll their own tcp socket using read/recvmsg etc.
> If they get handshake record, they forward it to the entity running handshake agent.
> The handshake agent could be a linux daemon or could run on a separate security
> processor like 'Secure element' or say arm trustzone etc. The applications
> forward any handshake message it gets backs from handshake agent to the
> connected tcp socket. Therefore, the applications act as a forwarder of the handshake
> messages between the peer tls endpoint and handshake agent.
> The received data messages are absorbed by the applications themselves (bypassing ssl stack
> completely). Similarly, the applications tx data directly by writing on their socket.
>
> > Waiting until after handshake solves both of these issues.
>
> The security sensitive check which is 'Wait for handshake to finish completely before
> accepting data' should not be the onus of the application. We have enough examples
> in past where application programmers made mistakes in setting up tls correctly. The idea
> is to isolate tls session setting up from the applications.
It's not clear to me what you gain by putting this 'handshake
finished' notification in the kernel instead of in the client's tls
library - you're already forwarding the handshake start notification
to the daemon, why can't the daemon notify them back in userspace that
the handshake is finished?
If you did want to put the notification in the kernel, how would you
handle poll on the socket, since probably both the handshake daemon
and client might be polling the socket, but one for control messages
and one for data?
The original kernel TLS RFC did split these to two separate sockets,
but we decided it was too complicated, and that's not how userspace
TLS clients function today.
Do you have an implementation of this? There are a bunch of tricky
corner cases here, it might make more sense to have something concrete
to discuss.
> Further, as per tls RFC it is ok to piggyback the data records after the finished handshake
> message. This is called early data. But then it is the responsibility of applications to first
> complete finished message processing before accepting the data records.
>
> The proposal is to disallow application world seeing data records
> before handshake finishes.
You're talking about the TLS 1.3 0-RTT feature, which is indeed an
interesting case. For in-process TLS libraries, it's fairly easy to
punt, and don't set the kernel TLS keys until after the 0-RTT data +
handshake message. For an OOB handshake daemon it might indeed make
more sense to leave the data in kernelspace ... somehow.
> > > - The handshake state should fallback to 'unverified' in case a control
> > record is seen again by kernel TLS (e.g. in case of renegotiation, post
> > handshake client auth etc).
> >
> > Currently kernel tls sockets return an error unless you explicitly handle the
> > control record for exactly this reason.
>
> IIRC, any kind handshake message post handshake-completion is a problem for kernel tls.
> This includes renegotiation, post handshake client-auth etc.
>
> Please correct me if I am wrong.
You are correct, but currently kernel TLS sockets return an error
unless you explicitly handle the control message. This should be
enough already to implement your proposal.
^ permalink raw reply
* Re: [net-next 10/16] net/mlx5: Support PCIe buffer congestion handling via Devlink
From: Jakub Kicinski @ 2018-07-30 22:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: Bjorn Helgaas, Moshe Shemesh, Jiri Pirko, Eran Ben Elisha,
Saeed Mahameed, David S. Miller, netdev@vger.kernel.org,
linux-pci
In-Reply-To: <CAKgT0UcbXT6EXt=zwoOzZ-wm7uS70wvm9xMcsoSf6OzuJsLxMQ@mail.gmail.com>
On Mon, 30 Jul 2018 08:02:48 -0700, Alexander Duyck wrote:
> On Mon, Jul 30, 2018 at 7:07 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Sun, Jul 29, 2018 at 03:00:28PM -0700, Alexander Duyck wrote:
> >> On Sun, Jul 29, 2018 at 2:23 AM, Moshe Shemesh <moshes20.il@gmail.com> wrote:
> >> > On Sat, Jul 28, 2018 at 7:06 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> >> On Thu, Jul 26, 2018 at 07:00:20AM -0700, Alexander Duyck wrote:
> >> >> > On Thu, Jul 26, 2018 at 12:14 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> >> >> > > Thu, Jul 26, 2018 at 02:43:59AM CEST, jakub.kicinski@netronome.com wrote:
> >> >> > >>On Wed, 25 Jul 2018 08:23:26 -0700, Alexander Duyck wrote:
> >> >> > >>> On Wed, Jul 25, 2018 at 5:31 AM, Eran Ben Elisha wrote:
> >> >> > >>> > On 7/24/2018 10:51 PM, Jakub Kicinski wrote:
> >> >> > >>> >>>> The devlink params haven't been upstream even for a full cycle
> >> >> > >>> >>>> and
> >> >> > >>> >>>> already you guys are starting to use them to configure standard
> >> >> > >>> >>>> features like queuing.
> >> >> > >>> >>>
> >> >> > >>> >>> We developed the devlink params in order to support non-standard
> >> >> > >>> >>> configuration only. And for non-standard, there are generic and
> >> >> > >>> >>> vendor
> >> >> > >>> >>> specific options.
> >> >> > >>> >>
> >> >> > >>> >> I thought it was developed for performing non-standard and
> >> >> > >>> >> possibly
> >> >> > >>> >> vendor specific configuration. Look at DEVLINK_PARAM_GENERIC_*
> >> >> > >>> >> for
> >> >> > >>> >> examples of well justified generic options for which we have no
> >> >> > >>> >> other API. The vendor mlx4 options look fairly vendor specific
> >> >> > >>> >> if you
> >> >> > >>> >> ask me, too.
> >> >> > >>> >>
> >> >> > >>> >> Configuring queuing has an API. The question is it acceptable to
> >> >> > >>> >> enter
> >> >> > >>> >> into the risky territory of controlling offloads via devlink
> >> >> > >>> >> parameters
> >> >> > >>> >> or would we rather make vendors take the time and effort to model
> >> >> > >>> >> things to (a subset) of existing APIs. The HW never fits the
> >> >> > >>> >> APIs
> >> >> > >>> >> perfectly.
> >> >> > >>> >
> >> >> > >>> > I understand what you meant here, I would like to highlight that
> >> >> > >>> > this
> >> >> > >>> > mechanism was not meant to handle SRIOV, Representors, etc.
> >> >> > >>> > The vendor specific configuration suggested here is to handle a
> >> >> > >>> > congestion
> >> >> > >>> > state in Multi Host environment (which includes PF and multiple
> >> >> > >>> > VFs per
> >> >> > >>> > host), where one host is not aware to the other hosts, and each is
> >> >> > >>> > running
> >> >> > >>> > on its own pci/driver. It is a device working mode configuration.
> >> >> > >>> >
> >> >> > >>> > This couldn't fit into any existing API, thus creating this
> >> >> > >>> > vendor specific
> >> >> > >>> > unique API is needed.
> >> >> > >>>
> >> >> > >>> If we are just going to start creating devlink interfaces in for
> >> >> > >>> every
> >> >> > >>> one-off option a device wants to add why did we even bother with
> >> >> > >>> trying to prevent drivers from using sysfs? This just feels like we
> >> >> > >>> are back to the same arguments we had back in the day with it.
> >> >> > >>>
> >> >> > >>> I feel like the bigger question here is if devlink is how we are
> >> >> > >>> going
> >> >> > >>> to deal with all PCIe related features going forward, or should we
> >> >> > >>> start looking at creating a new interface/tool for PCI/PCIe related
> >> >> > >>> features? My concern is that we have already had features such as
> >> >> > >>> DMA
> >> >> > >>> Coalescing that didn't really fit into anything and now we are
> >> >> > >>> starting to see other things related to DMA and PCIe bus credits.
> >> >> > >>> I'm
> >> >> > >>> wondering if we shouldn't start looking at a tool/interface to
> >> >> > >>> configure all the PCIe related features such as interrupts, error
> >> >> > >>> reporting, DMA configuration, power management, etc. Maybe we could
> >> >> > >>> even look at sharing it across subsystems and include things like
> >> >> > >>> storage, graphics, and other subsystems in the conversation.
> >> >> > >>
> >> >> > >>Agreed, for actual PCIe configuration (i.e. not ECN marking) we do
> >> >> > >> need
> >> >> > >>to build up an API. Sharing it across subsystems would be very cool!
> >> >>
> >> >> I read the thread (starting at [1], for anybody else coming in late)
> >> >> and I see this has something to do with "configuring outbound PCIe
> >> >> buffers", but I haven't seen the connection to PCIe protocol or
> >> >> features, i.e., I can't connect this to anything in the PCIe spec.
> >> >>
> >> >> Can somebody help me understand how the PCI core is relevant? If
> >> >> there's some connection with a feature defined by PCIe, or if it
> >> >> affects the PCIe transaction protocol somehow, I'm definitely
> >> >> interested in this. But if this only affects the data transferred
> >> >> over PCIe, i.e., the data payloads of PCIe TLP packets, then I'm not
> >> >> sure why the PCI core should care.
> >> >>
> >> >
> >> >
> >> > As you wrote, this is not a PCIe feature or affects the PCIe transaction
> >> > protocol.
> >> >
> >> > Actually, due to hardware limitation in current device, we have enabled a
> >> > workaround in hardware.
> >> >
> >> > This mode is proprietary and not relevant to other PCIe devices, thus is set
> >> > using driver-specific parameter in devlink
> >>
> >> Essentially what this feature is doing is communicating the need for
> >> PCIe back-pressure to the network fabric. So as the buffers on the
> >> device start to fill because the device isn't able to get back PCIe
> >> credits fast enough it will then start to send congestion
> >> notifications to the network stack itself if I understand this
> >> correctly.
> >
> > This sounds like a hook that allows the device to tell its driver
> > about PCIe flow control credits, and the driver can pass that on to
> > the network stack. IIUC, that would be a device-specific feature
> > outside the scope of the PCI core.
Hm, I might be wrong but AFAIU the patch which sparked the discussion
does not go all the way down to the PCIe FC. PCIe layer works at max
possible rate (single VC etc.), but there is a mismatch between network
and PCIe speed. E.g. with a 2x40GE NIC on a 8x8 PCIe v3 (63 Gbps) there
can be more traffic flowing in than PCIe bus will be able to transfer
to the host. From a networking ASIC perspective it's a fairly typical
problem of dealing with mismatched port speeds, incast, etc. GPUs or
storage devices will not have this problem, it will only happen with
non-flow controlled network technologies, i.e. netdevs. It so happens
the device is on a PCIe bus but the same problem can happen on SPI or
any other bus.
Having said that a PCIe configuration API seems to continue to come up.
Examples Alex gives below seem very valid (AFAIU them).
> >> For now there are no major conflicts, but when we start getting into
> >> stuff like PCIe DMA coalescing, and on a more general basis just PCIe
> >> active state power management that is going to start making things
> >> more complicated going forward.
> >
> > We do support ASPM already in the PCI core, and we do have the
> > pci_disable_link_state() interface, which is currently the only way
> > drivers can influence it. There are several drivers that do their own
> > ASPM configuration, but this is not safe because it's not coordinated
> > with what the PCI core does. If/when drivers need more control, we
> > should enhance the PCI core interfaces.
>
> This is kind of what I was getting at. It would be useful to have an
> interface of some sort so that drivers get notified when a user is
> making changes to configuration space and I don't know if anything
> like that exists now.
>
> > I don't know what PCIe DMA coalescing means, so I can't comment on
> > that.
>
> There are devices, specifically network devices, that will hold off on
> switching between either L0s or L1 and L0 by deferring DMA operations.
> Basically the idea is supposed to be to hold off bringing the link up
> for as long as possible in order to maximize power savings for the
> ASPM state. This is something that has come up in the past, and I
> don't know if there has been any interface determined for how to
> handle this sort of configuration. Most of it occurs through MMIO.
>
> >> I assume the devices we are talking about supporting this new feature
> >> on either don't deal with ASPM or assume a quick turnaround to get out
> >> of the lower power states? Otherwise that would definitely cause some
> >> back-pressure buildups that would hurt performance.
> >
> > Devices can communicate the ASPM exit latency they can tolerate via
> > the Device Capabilities register (PCIe r4.0, sec 76.5.3.3). Linux
> > should be configuring ASPM to respect those device requirements.
> >
> > Bjorn
>
> Right. But my point was something like ASPM will add extra complexity
> to a feature such as what has been described here. My concern is that
> I don't want us implementing stuff on a per-driver basis that is not
> all that unique to the device. I don't really see the feature that was
> described above as being something that will stay specific to this one
> device for very long, especially if it provides added value. Basically
> all it is doing is allowing exposing PCIe congestion management to
> upper levels in the network stack. I don't even necessarily see it as
> being networking specific as I would imagine there might be other
> types of devices that could make use of knowing how many transactions
> and such they could process at the same time.
>
> - Alex
^ permalink raw reply
* [PATCH net-next 0/4] tcp: add 4 new stats
From: Wei Wang @ 2018-07-30 21:59 UTC (permalink / raw)
To: David Miller, netdev
Cc: Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet,
Wei Wang
From: Wei Wang <weiwan@google.com>
This patch series adds 3 RFC4898 stats:
1. tcpEStatsPerfHCDataOctetsOut
2. tcpEStatsPerfOctetsRetrans
3. tcpEStatsStackDSACKDups
and an addtional stat to record the number of data packet reordering
events seen:
4. tcp_reord_seen
Together with the existing stats, application can use them to measure
the retransmission rate in bytes, exclude spurious retransmissions
reflected by DSACK, and keep track of the reordering events on live
connections.
In particular the networks with different MTUs make bytes-based loss stats
more useful. Google servers have been using these stats for many years to
instrument transport and network performance.
Wei Wang (4):
tcp: add data bytes sent stats
tcp: add data bytes retransmitted stats
tcp: add dsack blocks received stats
tcp: add stat of data packet reordering events
include/linux/tcp.h | 13 +++++++++++--
include/uapi/linux/tcp.h | 10 +++++++++-
net/ipv4/tcp.c | 19 +++++++++++++++++--
net/ipv4/tcp_input.c | 4 +++-
net/ipv4/tcp_output.c | 2 ++
net/ipv4/tcp_recovery.c | 2 +-
6 files changed, 43 insertions(+), 7 deletions(-)
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply
* [PATCH net-next 1/4] tcp: add data bytes sent stats
From: Wei Wang @ 2018-07-30 21:59 UTC (permalink / raw)
To: David Miller, netdev
Cc: Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet,
Wei Wang
In-Reply-To: <20180730215911.209895-1-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Introduce a new TCP stat to record the number of bytes sent
(RFC4898 tcpEStatsPerfHCDataOctetsOut) and expose it in both tcp_info
(TCP_INFO) and opt_stats (SOF_TIMESTAMPING_OPT_STATS).
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
include/linux/tcp.h | 3 +++
include/uapi/linux/tcp.h | 4 +++-
net/ipv4/tcp.c | 7 ++++++-
net/ipv4/tcp_output.c | 1 +
4 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 58a8d7d71354..d0798dcd2cab 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -181,6 +181,9 @@ struct tcp_sock {
u32 data_segs_out; /* RFC4898 tcpEStatsPerfDataSegsOut
* total number of data segments sent.
*/
+ u64 bytes_sent; /* RFC4898 tcpEStatsPerfHCDataOctetsOut
+ * total number of data bytes sent.
+ */
u64 bytes_acked; /* RFC4898 tcpEStatsAppHCThruOctetsAcked
* sum(delta(snd_una)), or how many bytes
* were acked.
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index e3f6ed8a7064..1c70ed287c3b 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -235,6 +235,8 @@ struct tcp_info {
__u32 tcpi_delivered;
__u32 tcpi_delivered_ce;
+
+ __u64 tcpi_bytes_sent; /* RFC4898 tcpEStatsPerfHCDataOctetsOut */
};
/* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */
@@ -257,7 +259,7 @@ enum {
TCP_NLA_SND_SSTHRESH, /* Slow start size threshold */
TCP_NLA_DELIVERED, /* Data pkts delivered incl. out-of-order */
TCP_NLA_DELIVERED_CE, /* Like above but only ones w/ CE marks */
-
+ TCP_NLA_BYTES_SENT, /* Data bytes sent including retransmission */
};
/* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index f3bfb9f29520..53d3db4a3d39 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2594,6 +2594,7 @@ int tcp_disconnect(struct sock *sk, int flags)
sk->sk_rx_dst = NULL;
tcp_saved_syn_free(tp);
tp->compressed_ack = 0;
+ tp->bytes_sent = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
@@ -3201,6 +3202,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
info->tcpi_delivery_rate = rate64;
info->tcpi_delivered = tp->delivered;
info->tcpi_delivered_ce = tp->delivered_ce;
+ info->tcpi_bytes_sent = tp->bytes_sent;
unlock_sock_fast(sk, slow);
}
EXPORT_SYMBOL_GPL(tcp_get_info);
@@ -3213,7 +3215,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
u64 rate64;
u32 rate;
- stats = alloc_skb(7 * nla_total_size_64bit(sizeof(u64)) +
+ stats = alloc_skb(8 * nla_total_size_64bit(sizeof(u64)) +
7 * nla_total_size(sizeof(u32)) +
3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
if (!stats)
@@ -3251,6 +3253,9 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
nla_put_u32(stats, TCP_NLA_SNDQ_SIZE, tp->write_seq - tp->snd_una);
nla_put_u8(stats, TCP_NLA_CA_STATE, inet_csk(sk)->icsk_ca_state);
+ nla_put_u64_64bit(stats, TCP_NLA_BYTES_SENT, tp->bytes_sent,
+ TCP_NLA_PAD);
+
return stats;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 490df62f26d4..861531fe0e97 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1136,6 +1136,7 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
if (skb->len != tcp_header_size) {
tcp_event_data_sent(tp, sk);
tp->data_segs_out += tcp_skb_pcount(skb);
+ tp->bytes_sent += skb->len - tcp_header_size;
tcp_internal_pacing(sk, skb);
}
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related
* [PATCH net-next 2/4] tcp: add data bytes retransmitted stats
From: Wei Wang @ 2018-07-30 21:59 UTC (permalink / raw)
To: David Miller, netdev
Cc: Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet,
Wei Wang
In-Reply-To: <20180730215911.209895-2-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Introduce a new TCP stat to record the number of bytes retransmitted
(RFC4898 tcpEStatsPerfOctetsRetrans) and expose it in both tcp_info
(TCP_INFO) and opt_stats (SOF_TIMESTAMPING_OPT_STATS).
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
include/linux/tcp.h | 3 +++
include/uapi/linux/tcp.h | 2 ++
net/ipv4/tcp.c | 6 +++++-
net/ipv4/tcp_output.c | 1 +
4 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d0798dcd2cab..fb67f9a51b95 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -333,6 +333,9 @@ struct tcp_sock {
* the first SYN. */
u32 undo_marker; /* snd_una upon a new recovery episode. */
int undo_retrans; /* number of undoable retransmissions. */
+ u64 bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans
+ * Total data bytes retransmitted
+ */
u32 total_retrans; /* Total retransmits for entire connection */
u32 urg_seq; /* Seq of received urgent pointer */
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 1c70ed287c3b..c31f5100b744 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -237,6 +237,7 @@ struct tcp_info {
__u32 tcpi_delivered_ce;
__u64 tcpi_bytes_sent; /* RFC4898 tcpEStatsPerfHCDataOctetsOut */
+ __u64 tcpi_bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans */
};
/* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */
@@ -260,6 +261,7 @@ enum {
TCP_NLA_DELIVERED, /* Data pkts delivered incl. out-of-order */
TCP_NLA_DELIVERED_CE, /* Like above but only ones w/ CE marks */
TCP_NLA_BYTES_SENT, /* Data bytes sent including retransmission */
+ TCP_NLA_BYTES_RETRANS, /* Data bytes retransmitted */
};
/* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 53d3db4a3d39..372357a035e9 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2595,6 +2595,7 @@ int tcp_disconnect(struct sock *sk, int flags)
tcp_saved_syn_free(tp);
tp->compressed_ack = 0;
tp->bytes_sent = 0;
+ tp->bytes_retrans = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
@@ -3203,6 +3204,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
info->tcpi_delivered = tp->delivered;
info->tcpi_delivered_ce = tp->delivered_ce;
info->tcpi_bytes_sent = tp->bytes_sent;
+ info->tcpi_bytes_retrans = tp->bytes_retrans;
unlock_sock_fast(sk, slow);
}
EXPORT_SYMBOL_GPL(tcp_get_info);
@@ -3215,7 +3217,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
u64 rate64;
u32 rate;
- stats = alloc_skb(8 * nla_total_size_64bit(sizeof(u64)) +
+ stats = alloc_skb(9 * nla_total_size_64bit(sizeof(u64)) +
7 * nla_total_size(sizeof(u32)) +
3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
if (!stats)
@@ -3255,6 +3257,8 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
nla_put_u64_64bit(stats, TCP_NLA_BYTES_SENT, tp->bytes_sent,
TCP_NLA_PAD);
+ nla_put_u64_64bit(stats, TCP_NLA_BYTES_RETRANS, tp->bytes_retrans,
+ TCP_NLA_PAD);
return stats;
}
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 861531fe0e97..50cabf7656f3 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2871,6 +2871,7 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
if (TCP_SKB_CB(skb)->tcp_flags & TCPHDR_SYN)
__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPSYNRETRANS);
tp->total_retrans += segs;
+ tp->bytes_retrans += skb->len;
/* make sure skb->data is aligned on arches that require it
* and check if ack-trimming & collapsing extended the headroom
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related
* [PATCH net-next 3/4] tcp: add dsack blocks received stats
From: Wei Wang @ 2018-07-30 21:59 UTC (permalink / raw)
To: David Miller, netdev
Cc: Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet,
Wei Wang
In-Reply-To: <20180730215911.209895-3-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Introduce a new TCP stat to record the number of DSACK blocks received
(RFC4989 tcpEStatsStackDSACKDups) and expose it in both tcp_info
(TCP_INFO) and opt_stats (SOF_TIMESTAMPING_OPT_STATS).
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
include/linux/tcp.h | 3 +++
include/uapi/linux/tcp.h | 2 ++
net/ipv4/tcp.c | 5 ++++-
net/ipv4/tcp_input.c | 1 +
4 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index fb67f9a51b95..da6281c549a5 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -188,6 +188,9 @@ struct tcp_sock {
* sum(delta(snd_una)), or how many bytes
* were acked.
*/
+ u32 dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups
+ * total number of DSACK blocks received
+ */
u32 snd_una; /* First byte we want an ack for */
u32 snd_sml; /* Last byte of the most recently transmitted small packet */
u32 rcv_tstamp; /* timestamp of last received ACK (for keepalives) */
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index c31f5100b744..0e1c0aec0153 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -238,6 +238,7 @@ struct tcp_info {
__u64 tcpi_bytes_sent; /* RFC4898 tcpEStatsPerfHCDataOctetsOut */
__u64 tcpi_bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans */
+ __u32 tcpi_dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups */
};
/* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */
@@ -262,6 +263,7 @@ enum {
TCP_NLA_DELIVERED_CE, /* Like above but only ones w/ CE marks */
TCP_NLA_BYTES_SENT, /* Data bytes sent including retransmission */
TCP_NLA_BYTES_RETRANS, /* Data bytes retransmitted */
+ TCP_NLA_DSACK_DUPS, /* DSACK blocks received */
};
/* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 372357a035e9..a8ec6564a7ec 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2596,6 +2596,7 @@ int tcp_disconnect(struct sock *sk, int flags)
tp->compressed_ack = 0;
tp->bytes_sent = 0;
tp->bytes_retrans = 0;
+ tp->dsack_dups = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
@@ -3205,6 +3206,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
info->tcpi_delivered_ce = tp->delivered_ce;
info->tcpi_bytes_sent = tp->bytes_sent;
info->tcpi_bytes_retrans = tp->bytes_retrans;
+ info->tcpi_dsack_dups = tp->dsack_dups;
unlock_sock_fast(sk, slow);
}
EXPORT_SYMBOL_GPL(tcp_get_info);
@@ -3218,7 +3220,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
u32 rate;
stats = alloc_skb(9 * nla_total_size_64bit(sizeof(u64)) +
- 7 * nla_total_size(sizeof(u32)) +
+ 8 * nla_total_size(sizeof(u32)) +
3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
if (!stats)
return NULL;
@@ -3259,6 +3261,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
TCP_NLA_PAD);
nla_put_u64_64bit(stats, TCP_NLA_BYTES_RETRANS, tp->bytes_retrans,
TCP_NLA_PAD);
+ nla_put_u32(stats, TCP_NLA_DSACK_DUPS, tp->dsack_dups);
return stats;
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d51fa358b2b1..fbc85ff7d71d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -874,6 +874,7 @@ static void tcp_dsack_seen(struct tcp_sock *tp)
{
tp->rx_opt.sack_ok |= TCP_DSACK_SEEN;
tp->rack.dsack_seen = 1;
+ tp->dsack_dups++;
}
/* It's reordering when higher sequence was delivered (i.e. sacked) before
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related
* [PATCH net-next 4/4] tcp: add stat of data packet reordering events
From: Wei Wang @ 2018-07-30 21:59 UTC (permalink / raw)
To: David Miller, netdev
Cc: Yuchung Cheng, Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet,
Wei Wang
In-Reply-To: <20180730215911.209895-4-tracywwnj@gmail.com>
From: Wei Wang <weiwan@google.com>
Introduce a new TCP stats to record the number of reordering events seen
and expose it in both tcp_info (TCP_INFO) and opt_stats
(SOF_TIMESTAMPING_OPT_STATS).
Application can use this stats to track the frequency of the reordering
events in addition to the existing reordering stats which tracks the
magnitude of the latest reordering event.
Note: this new stats tracks reordering events triggered by ACKs, which
could often be fewer than the actual number of packets being delivered
out-of-order.
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
---
include/linux/tcp.h | 4 ++--
include/uapi/linux/tcp.h | 2 ++
net/ipv4/tcp.c | 5 ++++-
net/ipv4/tcp_input.c | 3 ++-
net/ipv4/tcp_recovery.c | 2 +-
5 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index da6281c549a5..263e37271afd 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -220,8 +220,7 @@ struct tcp_sock {
#define TCP_RACK_RECOVERY_THRESH 16
u8 reo_wnd_persist:5, /* No. of recovery since last adj */
dsack_seen:1, /* Whether DSACK seen after last adj */
- advanced:1, /* mstamp advanced since last lost marking */
- reord:1; /* reordering detected */
+ advanced:1; /* mstamp advanced since last lost marking */
} rack;
u16 advmss; /* Advertised MSS */
u8 compressed_ack;
@@ -267,6 +266,7 @@ struct tcp_sock {
u8 ecn_flags; /* ECN status bits. */
u8 keepalive_probes; /* num of allowed keep alive probes */
u32 reordering; /* Packet reordering metric. */
+ u32 reord_seen; /* number of data packet reordering events */
u32 snd_up; /* Urgent pointer */
/*
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 0e1c0aec0153..e02d31986ff9 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -239,6 +239,7 @@ struct tcp_info {
__u64 tcpi_bytes_sent; /* RFC4898 tcpEStatsPerfHCDataOctetsOut */
__u64 tcpi_bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans */
__u32 tcpi_dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups */
+ __u32 tcpi_reord_seen; /* reordering events seen */
};
/* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */
@@ -264,6 +265,7 @@ enum {
TCP_NLA_BYTES_SENT, /* Data bytes sent including retransmission */
TCP_NLA_BYTES_RETRANS, /* Data bytes retransmitted */
TCP_NLA_DSACK_DUPS, /* DSACK blocks received */
+ TCP_NLA_REORD_SEEN, /* reordering events seen */
};
/* for TCP_MD5SIG socket option */
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a8ec6564a7ec..250b73ccb644 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2597,6 +2597,7 @@ int tcp_disconnect(struct sock *sk, int flags)
tp->bytes_sent = 0;
tp->bytes_retrans = 0;
tp->dsack_dups = 0;
+ tp->reord_seen = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
@@ -3207,6 +3208,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info)
info->tcpi_bytes_sent = tp->bytes_sent;
info->tcpi_bytes_retrans = tp->bytes_retrans;
info->tcpi_dsack_dups = tp->dsack_dups;
+ info->tcpi_reord_seen = tp->reord_seen;
unlock_sock_fast(sk, slow);
}
EXPORT_SYMBOL_GPL(tcp_get_info);
@@ -3220,7 +3222,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
u32 rate;
stats = alloc_skb(9 * nla_total_size_64bit(sizeof(u64)) +
- 8 * nla_total_size(sizeof(u32)) +
+ 9 * nla_total_size(sizeof(u32)) +
3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
if (!stats)
return NULL;
@@ -3262,6 +3264,7 @@ struct sk_buff *tcp_get_timestamping_opt_stats(const struct sock *sk)
nla_put_u64_64bit(stats, TCP_NLA_BYTES_RETRANS, tp->bytes_retrans,
TCP_NLA_PAD);
nla_put_u32(stats, TCP_NLA_DSACK_DUPS, tp->dsack_dups);
+ nla_put_u32(stats, TCP_NLA_REORD_SEEN, tp->reord_seen);
return stats;
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index fbc85ff7d71d..3d6156f07a8d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -906,8 +906,8 @@ static void tcp_check_sack_reordering(struct sock *sk, const u32 low_seq,
sock_net(sk)->ipv4.sysctl_tcp_max_reordering);
}
- tp->rack.reord = 1;
/* This exciting event is worth to be remembered. 8) */
+ tp->reord_seen++;
NET_INC_STATS(sock_net(sk),
ts ? LINUX_MIB_TCPTSREORDER : LINUX_MIB_TCPSACKREORDER);
}
@@ -1871,6 +1871,7 @@ static void tcp_check_reno_reordering(struct sock *sk, const int addend)
tp->reordering = min_t(u32, tp->packets_out + addend,
sock_net(sk)->ipv4.sysctl_tcp_max_reordering);
+ tp->reord_seen++;
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRENOREORDER);
}
diff --git a/net/ipv4/tcp_recovery.c b/net/ipv4/tcp_recovery.c
index 71593e4400ab..c81aadff769b 100644
--- a/net/ipv4/tcp_recovery.c
+++ b/net/ipv4/tcp_recovery.c
@@ -25,7 +25,7 @@ static u32 tcp_rack_reo_wnd(const struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
- if (!tp->rack.reord) {
+ if (!tp->reord_seen) {
/* If reordering has not been observed, be aggressive during
* the recovery or starting the recovery by DUPACK threshold.
*/
--
2.18.0.345.g5c9ce644c3-goog
^ permalink raw reply related
* Re: [PATCH net-next 2/4] tcp: add data bytes retransmitted stats
From: Stephen Hemminger @ 2018-07-30 22:14 UTC (permalink / raw)
To: Wei Wang
Cc: David Miller, netdev, Yuchung Cheng, Neal Cardwell,
Soheil Hassas Yeganeh, Eric Dumazet
In-Reply-To: <20180730215911.209895-3-tracywwnj@gmail.com>
On Mon, 30 Jul 2018 14:59:09 -0700
Wei Wang <weiwan@google.com> wrote:
> + stats = alloc_skb(9 * nla_total_size_64bit(sizeof(u64)) +
> 7 * nla_total_size(sizeof(u32)) +
> 3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
This is getting a bit awkward.
Maybe use the style used in other drivers that have a get_size function
like tun_get_size().
^ permalink raw reply
* Re: [PATCH bpf-next] lwt_bpf: remove unnecessary rcu_read_lock in run_lwt_bpf
From: Y Song @ 2018-07-30 22:22 UTC (permalink / raw)
To: Taehee Yoo; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev
In-Reply-To: <20180730132259.6338-1-ap420073@gmail.com>
On Mon, Jul 30, 2018 at 6:22 AM, Taehee Yoo <ap420073@gmail.com> wrote:
> run_lwt_bpf is called by bpf_{input/output/xmit}.
> These functions are already protected by rcu_read_lock.
> because lwtunnel_{input/output/xmit} holds rcu_read_lock
> and then calls bpf_{input/output/xmit}.
> So that rcu_read_lock in the run_lwt_bpf is unnecessary.
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Florian Fainelli @ 2018-07-30 22:26 UTC (permalink / raw)
To: netdev; +Cc: linville, davem, andrew, vivien.didelot
In-Reply-To: <20180717153645.7500-2-f.fainelli@gmail.com>
On 07/17/2018 08:36 AM, Florian Fainelli wrote:
> Allow re-purposing the wol->sopass storage area to specify a bitmask of filters
> (programmed previously via ethtool::rxnfc) to be used as wake-up patterns.
John, David, can you provide some feedback if the approach is
acceptable? I will address Andrew's comment about the user friendliness
and allow providing a comma separate list of filter identifiers.
One usability issue with this approach is that one cannot specify
wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time,
since it uses the same location in the ioctl() structure that is being
passed. Do you see this as a problem?
Thanks!
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> ethtool-copy.h | 1 +
> ethtool.c | 35 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/ethtool-copy.h b/ethtool-copy.h
> index 8cc61e9ab40b..dbfaca15dca5 100644
> --- a/ethtool-copy.h
> +++ b/ethtool-copy.h
> @@ -1628,6 +1628,7 @@ static __inline__ int ethtool_validate_duplex(__u8 duplex)
> #define WAKE_ARP (1 << 4)
> #define WAKE_MAGIC (1 << 5)
> #define WAKE_MAGICSECURE (1 << 6) /* only meaningful if WAKE_MAGIC */
> +#define WAKE_FILTER (1 << 7)
>
> /* L2-L4 network traffic flow types */
> #define TCP_V4_FLOW 0x01 /* hash or spec (tcp_ip4_spec) */
> diff --git a/ethtool.c b/ethtool.c
> index fb93ae898312..322fc8d98ee5 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -931,6 +931,9 @@ static int parse_wolopts(char *optstr, u32 *data)
> case 's':
> *data |= WAKE_MAGICSECURE;
> break;
> + case 'f':
> + *data |= WAKE_FILTER;
> + break;
> case 'd':
> *data = 0;
> break;
> @@ -964,6 +967,8 @@ static char *unparse_wolopts(int wolopts)
> *p++ = 'g';
> if (wolopts & WAKE_MAGICSECURE)
> *p++ = 's';
> + if (wolopts & WAKE_FILTER)
> + *p++ = 'f';
> } else {
> *p = 'd';
> }
> @@ -989,6 +994,21 @@ static int dump_wol(struct ethtool_wolinfo *wol)
> fprintf(stdout, "\n");
> }
>
> + if (wol->supported & WAKE_FILTER) {
> + int i, j;
> + int delim = 0;
> + fprintf(stdout, " Filter(s) enabled: ");
> + for (i = 0; i < SOPASS_MAX; i++) {
> + for (j = 0; j < 8; j++) {
> + if (wol->sopass[i] & (1 << j)) {
> + fprintf(stdout, "%s%d", delim?",":"", i * 8 + j);
> + delim=1;
> + }
> + }
> + }
> + fprintf(stdout, "\n");
> + }
> +
> return 0;
> }
>
> @@ -2897,6 +2917,16 @@ static int do_sset(struct cmd_context *ctx)
> exit_bad_args();
> get_mac_addr(argp[i], sopass_wanted);
> sopass_change = 1;
> + } else if (!strcmp(argp[i], "filters")) {
> + gwol_changed = 1;
> + i++;
> + if (i >= argc)
> + exit_bad_args();
> + if (parse_hex_u32_bitmap(argp[i],
> + SOPASS_MAX * 8,
> + (unsigned int *)sopass_wanted))
> + exit_bad_args();
> + sopass_change = 1;
> } else if (!strcmp(argp[i], "msglvl")) {
> i++;
> if (i >= argc)
> @@ -3112,8 +3142,10 @@ static int do_sset(struct cmd_context *ctx)
> if (err < 0) {
> if (wol_change)
> fprintf(stderr, " not setting wol\n");
> - if (sopass_change)
> + if (sopass_change & wol.wolopts & WAKE_MAGICSECURE)
> fprintf(stderr, " not setting sopass\n");
> + if (sopass_change & wol.wolopts & WAKE_FILTER)
> + fprintf(stderr, " not setting filters\n");
> }
> }
>
> @@ -5066,6 +5098,7 @@ static const struct option {
> " [ xcvr internal|external ]\n"
> " [ wol p|u|m|b|a|g|s|d... ]\n"
> " [ sopass %x:%x:%x:%x:%x:%x ]\n"
> + " [ filters %x ]\n"
> " [ msglvl %d | msglvl type on|off ... ]\n" },
> { "-a|--show-pause", 1, do_gpause, "Show pause options" },
> { "-A|--pause", 1, do_spause, "Set pause options",
>
--
Florian
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Andrew Lunn @ 2018-07-30 22:30 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, linville, davem, vivien.didelot
In-Reply-To: <47f52797-1540-5eeb-5ea8-3c0984fdfb11@gmail.com>
> One usability issue with this approach is that one cannot specify
> wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time,
> since it uses the same location in the ioctl() structure that is being
> passed. Do you see this as a problem?
Hi Florian
Maybe you can think ahead to when ethtool gets a netlink
implementation, and you have more flexibility to pass both as
attributes? How does this affect the in kernel API?
Andrew
^ permalink raw reply
* Re: [PATCH v3] perf build: Build error in libbpf with EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
From: Daniel Borkmann @ 2018-07-31 0:13 UTC (permalink / raw)
To: Jakub Kicinski, Thomas Richter
Cc: ast, netdev, linux-kernel, heiko.carstens, brueckner, schwidefsky
In-Reply-To: <20180730123509.0ffa4737@cakuba.netronome.com>
On 07/30/2018 09:35 PM, Jakub Kicinski wrote:
> On Mon, 30 Jul 2018 10:53:23 +0200, Thomas Richter wrote:
>> commit 531b014e7a2f ("tools: bpf: make use of reallocarray")
>> causes a compiler error when building the perf tool in the linux-next
>> tree. Compile file tools/lib/bpf/libbpf.c on a FEDORA 28
>> installation with gcc compiler
>> version: gcc (GCC) 8.0.1 20180324 (Red Hat 8.0.1-0.20)
>> shows this error message:
>>
>> [root@p23lp27] # make V=1 EXTRA_CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -O2"
>> [...]
>> make -f /home6/tmricht/linux-next/tools/build/Makefile.build
>> dir=./util/scripting-engines obj=libperf
>> libbpf.c: In function ‘bpf_object__elf_collect’:
>> libbpf.c:811:15: error: ignoring return value of ‘strerror_r’,
>> declared with attribute warn_unused_result [-Werror=unused-result]
>> strerror_r(-err, errmsg, sizeof(errmsg));
>> ^
>> cc1: all warnings being treated as errors
>> mv: cannot stat './.libbpf.o.tmp': No such file or directory
>> /home6/tmricht/linux-next/tools/build/Makefile.build:96: recipe for target 'libbpf.o' failed
>>
>> Replace all occurrences of strerror() by calls to strerror_r().
>> To keep the compiler quiet also use the return value from strerror_r()
>> otherwise a 'variable set but not use' warning which is treated as error
>> terminates the compile.
>>
>> Cc: Alexei Starovoitov <ast@kernel.org>
>> Cc: Daniel Borkmann <daniel@iogearbox.net>
>> Cc: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Suggested-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> Suggested-by: Daniel Borkmann <daniel@iogearbox.net>
>> Reviewed-by: Hendrik Brueckner <brueckner@linux.ibm.com>
>> Signed-off-by: Thomas Richter <tmricht@linux.ibm.com>
Applied to bpf-next, thanks Thomas!
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Florian Fainelli @ 2018-07-30 22:39 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linville, davem, vivien.didelot
In-Reply-To: <20180730223045.GB10559@lunn.ch>
On 07/30/2018 03:30 PM, Andrew Lunn wrote:
>> One usability issue with this approach is that one cannot specify
>> wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time,
>> since it uses the same location in the ioctl() structure that is being
>> passed. Do you see this as a problem?
>
> Hi Florian
>
> Maybe you can think ahead to when ethtool gets a netlink
> implementation, and you have more flexibility to pass both as
> attributes? How does this affect the in kernel API?
The thing is that I need this now, but when Michal's work on ethtool
being migrated to netlink settles, we should have have any issues adding
a proper storage area for specifying filters anymore. The issue here is
of course that the size and layout of ethtool_wolinfo is largely fixed
and set in stone, and therefore inflexible.
--
Florian
^ permalink raw reply
* Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command
From: Jakub Kicinski @ 2018-07-31 0:26 UTC (permalink / raw)
To: Daniel Colascione
Cc: Daniel Borkmann, Joel Fernandes, linux-kernel, Tim Murray, netdev,
Alexei Starovoitov, Lorenzo Colitti, Chenbo Feng,
Mathieu Desnoyers, Alexei Starovoitov
In-Reply-To: <CAKOZuesH_LRdt15aXaRQRPSNzN-URTQWgSgr0zLsv8ZKFU9=9Q@mail.gmail.com>
On Mon, 30 Jul 2018 03:25:43 -0700, Daniel Colascione wrote:
> On Mon, Jul 30, 2018 at 3:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> > Hmm, I don't think such UAPI as above is future-proof. In case we would want
> > a similar mechanism in future for other maps, we would need a whole new bpf
> > command or reuse BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES as a workaround though
> > the underlying map may not even be a map-to-map. Additionally, we don't have
> > any map object at hand in the above, so we couldn't make any finer grained
> > decisions either. Something like below would be more suitable and leaves room
> > for extending this further in future.
>
> YAGNI. Your proposed mechanism doesn't add anything under the current
> implementation.
FWIW in case of HW offload targeting a particular map may allow users
to avoid a potentially slow sync with all the devices on the system.
> It's also not clear how a map-specific synchronization
> command is supposed to work in cases where we swap multiple map
> references. Do we synchronize_rcu multiple times? Why would we impose
> that inefficiency just for the sake of some non-specific future
> extensibility? Add some kind of batching layer? The current approach
> works for the anticipated use cases.
>
> While my preference is not to talk about map-to-maps at all in the
> user API and instead spec the thing as talking about map references in
> general, I'd rather have something that talks about
> references-to-maps-acquired-from-maps than this interface.
^ permalink raw reply
* Re: [PATCH v2 3/7] arm64: dts: Fix the base address of the Broadcom iProc mdio mux
From: Florian Fainelli @ 2018-07-30 22:50 UTC (permalink / raw)
To: Arun Parameswaran, David S. Miller, Florian Fainelli, Andrew Lunn,
Rob Herring, Mark Rutland, Ray Jui, Scott Branden,
Catalin Marinas, Will Deacon
Cc: netdev, devicetree, linux-arm-kernel, linux-kernel,
bcm-kernel-feedback-list
In-Reply-To: <1532726613-6483-4-git-send-email-arun.parameswaran@broadcom.com>
On 07/27/2018 02:23 PM, Arun Parameswaran wrote:
> Modify the base address of the mdio mux driver to point to the
> start of the mdio mux block's register address space.
>
> Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/4] tcp: add data bytes retransmitted stats
From: Wei Wang @ 2018-07-30 22:51 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S . Miller, Linux Kernel Network Developers, Yuchung Cheng,
Neal Cardwell, Soheil Hassas Yeganeh, Eric Dumazet
In-Reply-To: <20180730151414.4ce34375@xeon-e3>
On Mon, Jul 30, 2018 at 3:14 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 30 Jul 2018 14:59:09 -0700
> Wei Wang <weiwan@google.com> wrote:
>
> > + stats = alloc_skb(9 * nla_total_size_64bit(sizeof(u64)) +
> > 7 * nla_total_size(sizeof(u32)) +
> > 3 * nla_total_size(sizeof(u8)), GFP_ATOMIC);
>
> This is getting a bit awkward.
> Maybe use the style used in other drivers that have a get_size function
> like tun_get_size().
Thanks Stephen. OK. Will add a patch at the beginning to do this refactor.
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Andrew Lunn @ 2018-07-30 22:55 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, linville, davem, vivien.didelot
In-Reply-To: <ef6c3945-531a-b5d3-efa0-db3440d91d7b@gmail.com>
> The thing is that I need this now, but when Michal's work on ethtool
> being migrated to netlink settles, we should have have any issues adding
> a proper storage area for specifying filters anymore. The issue here is
> of course that the size and layout of ethtool_wolinfo is largely fixed
> and set in stone, and therefore inflexible.
The version in uapi/linux/ethtool.h is fixed. But i think in order to
implement this properly, you are going to have to change the internal
structure passed to ethtool_ops->set_wol/get_wol, with the filter
separated out from the sopass. ethtool_set_wol()/ethtool_get_wol()
would then need to know how to correctly unpack the structure passed
in the ioctl, where as the netlink socket code and just put attributes
into structure members.
Andrew
^ permalink raw reply
* Re: [PATCH v3] Add BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES bpf(2) command
From: Daniel Colascione @ 2018-07-31 0:33 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Joel Fernandes, linux-kernel, Tim Murray, netdev,
Alexei Starovoitov, Lorenzo Colitti, Chenbo Feng,
Mathieu Desnoyers, Alexei Starovoitov
In-Reply-To: <20180730172641.7d516231@cakuba.netronome.com>
On Mon, Jul 30, 2018 at 5:26 PM, Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Mon, 30 Jul 2018 03:25:43 -0700, Daniel Colascione wrote:
> > On Mon, Jul 30, 2018 at 3:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> > > Hmm, I don't think such UAPI as above is future-proof. In case we would want
> > > a similar mechanism in future for other maps, we would need a whole new bpf
> > > command or reuse BPF_SYNCHRONIZE_MAP_TO_MAP_REFERENCES as a workaround though
> > > the underlying map may not even be a map-to-map. Additionally, we don't have
> > > any map object at hand in the above, so we couldn't make any finer grained
> > > decisions either. Something like below would be more suitable and leaves room
> > > for extending this further in future.
> >
> > YAGNI. Your proposed mechanism doesn't add anything under the current
> > implementation.
>
> FWIW in case of HW offload targeting a particular map may allow users
> to avoid a potentially slow sync with all the devices on the system.
Sure. But such a thing doesn't exist right now (right?), and we can
add that more-efficient-in-that-one-case BPF interface when it lands.
I'd rather keep things simple for now.
^ permalink raw reply
* Re: [PATCH ethtool] ethtool: Add support for WAKE_FILTER
From: Andrew Lunn @ 2018-07-30 23:01 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, linville, davem, vivien.didelot
In-Reply-To: <47f52797-1540-5eeb-5ea8-3c0984fdfb11@gmail.com>
> One usability issue with this approach is that one cannot specify
> wake-on-LAN using WAKE_MAGICSECURE *and* WAKE_FILTER at the same time,
> since it uses the same location in the ioctl() structure that is being
> passed. Do you see this as a problem?
Hi Florian
I think you missed adding a check for this. ethtool_set_wol() should
check if both WAKE_FILTER and WAKE_MAGICSECURE are set and return
EINVAL.
Andrew
^ 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