* RE: [net-next v2 2/2] net: reject ptp requests with unsupported flags
From: Keller, Jacob E @ 2019-09-26 17:41 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev@vger.kernel.org, Intel Wired LAN, Kirsher, Jeffrey T,
Felipe Balbi, David S . Miller, Hall, Christopher S
In-Reply-To: <20190926040222.GB21883@localhost>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Richard Cochran
> Sent: Wednesday, September 25, 2019 9:02 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Felipe Balbi
> <felipe.balbi@linux.intel.com>; David S . Miller <davem@davemloft.net>; Hall,
> Christopher S <christopher.s.hall@intel.com>
> Subject: Re: [net-next v2 2/2] net: reject ptp requests with unsupported flags
>
> On Wed, Sep 25, 2019 at 07:28:20PM -0700, Jacob Keller wrote:
> > This patch may not be correct for individual drivers, especially
> > regarding the rising vs falling edge flags. I interpreted the default
> > behavior to be to timestamp the rising edge of a pin transition.
>
> So I think this patch goes too far. It breaks the implied ABI.
Sure, I didn't really know whether the rising vs falling edge and how it was supposed to work for each driver. I just want to ensure that any future flags get rejected until they are actually supported.
>
> > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> > index fd3071f55bd3..2867a2581a36 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> > @@ -521,6 +521,10 @@ static int igb_ptp_feature_enable_i210(struct
> ptp_clock_info *ptp,
> >
> > switch (rq->type) {
> > case PTP_CLK_REQ_EXTTS:
> > + /* Reject requests with unsupported flags */
> > + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE | PTP_RISING_EDGE))
> > + return -EOPNOTSUPP;
>
> This HW always time stamps both edges, and that is not configurable.
> Here you reject PTP_FALLING_EDGE, and that is clearly wrong. If the
> driver had been really picky (my fault I guess), it should have always
> insisted on (PTP_RISING_EDGE | PTP_FALLING_EDGE) being set together.
> But it is too late to enforce that now, because it could break user
> space programs.
Yes.
>
> I do agree with the sentiment of checking the flags at the driver
> level, but this needs to be done case by case, with the drivers'
> author's input.
Sure. I think the best immediate approach is to make sure all drivers reject any *new* flags, and each driver can decide whether they should reject rising, falling, etc.
>
> (The req.perout.flags can be done unconditionally in all drivers,
> since there were never any valid flags, but req.extts.flags needs
> careful attention.)
>
Right.
> Thanks,
> Richard
^ permalink raw reply
* RE: [net-next v2 2/2] net: reject ptp requests with unsupported flags
From: Keller, Jacob E @ 2019-09-26 17:42 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev@vger.kernel.org, Intel Wired LAN, Kirsher, Jeffrey T,
Felipe Balbi, David S . Miller, Hall, Christopher S
In-Reply-To: <20190926040222.GB21883@localhost>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Richard Cochran
> Sent: Wednesday, September 25, 2019 9:02 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: netdev@vger.kernel.org; Intel Wired LAN <intel-wired-lan@lists.osuosl.org>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Felipe Balbi
> <felipe.balbi@linux.intel.com>; David S . Miller <davem@davemloft.net>; Hall,
> Christopher S <christopher.s.hall@intel.com>
> Subject: Re: [net-next v2 2/2] net: reject ptp requests with unsupported flags
>
> On Wed, Sep 25, 2019 at 07:28:20PM -0700, Jacob Keller wrote:
> > This patch may not be correct for individual drivers, especially
> > regarding the rising vs falling edge flags. I interpreted the default
> > behavior to be to timestamp the rising edge of a pin transition.
>
> So I think this patch goes too far. It breaks the implied ABI.
>
> > diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c
> b/drivers/net/ethernet/intel/igb/igb_ptp.c
> > index fd3071f55bd3..2867a2581a36 100644
> > --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> > +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> > @@ -521,6 +521,10 @@ static int igb_ptp_feature_enable_i210(struct
> ptp_clock_info *ptp,
> >
> > switch (rq->type) {
> > case PTP_CLK_REQ_EXTTS:
> > + /* Reject requests with unsupported flags */
> > + if (rq->extts.flags & ~(PTP_ENABLE_FEATURE | PTP_RISING_EDGE))
> > + return -EOPNOTSUPP;
>
> This HW always time stamps both edges, and that is not configurable.
> Here you reject PTP_FALLING_EDGE, and that is clearly wrong. If the
> driver had been really picky (my fault I guess), it should have always
> insisted on (PTP_RISING_EDGE | PTP_FALLING_EDGE) being set together.
> But it is too late to enforce that now, because it could break user
> space programs.
>
> I do agree with the sentiment of checking the flags at the driver
> level, but this needs to be done case by case, with the drivers'
> author's input.
>
> (The req.perout.flags can be done unconditionally in all drivers,
> since there were never any valid flags, but req.extts.flags needs
> careful attention.)
>
> Thanks,
> Richard
I will split this patch apart and make the extts part per-driver.
Thanks,
Jake
^ permalink raw reply
* Re: [RFC 15/20] RDMA/irdma: Add miscellaneous utility definitions
From: Leon Romanovsky @ 2019-09-26 17:49 UTC (permalink / raw)
To: Jeff Kirsher
Cc: dledford, jgg, gregkh, Mustafa Ismail, netdev, linux-rdma,
Shiraz Saleem
In-Reply-To: <20190926164519.10471-16-jeffrey.t.kirsher@intel.com>
On Thu, Sep 26, 2019 at 09:45:14AM -0700, Jeff Kirsher wrote:
> From: Mustafa Ismail <mustafa.ismail@intel.com>
>
> Add miscellaneous utility functions and headers.
>
> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
> drivers/infiniband/hw/irdma/osdep.h | 108 ++
> drivers/infiniband/hw/irdma/protos.h | 96 ++
> drivers/infiniband/hw/irdma/status.h | 70 +
> drivers/infiniband/hw/irdma/utils.c | 2333 ++++++++++++++++++++++++++
> 4 files changed, 2607 insertions(+)
> create mode 100644 drivers/infiniband/hw/irdma/osdep.h
> create mode 100644 drivers/infiniband/hw/irdma/protos.h
> create mode 100644 drivers/infiniband/hw/irdma/status.h
> create mode 100644 drivers/infiniband/hw/irdma/utils.c
>
> diff --git a/drivers/infiniband/hw/irdma/osdep.h b/drivers/infiniband/hw/irdma/osdep.h
> new file mode 100644
> index 000000000000..5885b6fa413d
> --- /dev/null
> +++ b/drivers/infiniband/hw/irdma/osdep.h
> @@ -0,0 +1,108 @@
> +/* SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB */
> +/* Copyright (c) 2019, Intel Corporation. */
> +
> +#ifndef IRDMA_OSDEP_H
> +#define IRDMA_OSDEP_H
> +
> +#include <linux/version.h>
> +#include <linux/kernel.h>
> +#include <linux/vmalloc.h>
> +#include <linux/string.h>
> +#include <linux/bitops.h>
> +#include <linux/pci.h>
> +#include <net/tcp.h>
> +#include <crypto/hash.h>
> +/* get readq/writeq support for 32 bit kernels, use the low-first version */
> +#include <linux/io-64-nonatomic-lo-hi.h>
> +
> +#define MAKEMASK(m, s) ((m) << (s))
It is a little bit over-macro.
Thanks
^ permalink raw reply
* Re: [PATCH net] net: stmmac: dwmac-meson8b: Fix signedness bug in probe
From: Martin Blumenstingl @ 2019-09-26 17:51 UTC (permalink / raw)
To: Dan Carpenter
Cc: Giuseppe Cavallaro, Alexandre Torgue, Jose Abreu, David S. Miller,
Kevin Hilman, Maxime Coquelin, netdev, linux-stm32,
kernel-janitors, linux-amlogic
In-Reply-To: <20190925105822.GH3264@mwanda>
+Cc linux-amlogic mailing list
On Wed, Sep 25, 2019 at 12:59 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> The "dwmac->phy_mode" is an enum and in this context GCC treats it as
> an unsigned int so the error handling is never triggered.
>
> Fixes: 566e82516253 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
thank you for catching and fixing this!
Martin
^ permalink raw reply
* Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
From: gregkh @ 2019-09-26 18:02 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Jeff Kirsher, dledford@redhat.com, Mustafa Ismail,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Shiraz Saleem
In-Reply-To: <20190926165506.GF19509@mellanox.com>
On Thu, Sep 26, 2019 at 04:55:12PM +0000, Jason Gunthorpe wrote:
> On Thu, Sep 26, 2019 at 09:45:03AM -0700, Jeff Kirsher wrote:
> > +int i40iw_probe(struct platform_device *pdev)
> > +{
> > + struct i40e_peer_dev_platform_data *pdata =
> > + dev_get_platdata(&pdev->dev);
> > + struct i40e_info *ldev;
>
> I thought Greg already said not to use platform_device for this?
Yes I did, which is what I thought this whole "use MFD" was supposed to
solve. Why is a platform device still being used here?
thanks,
greg k-h
^ permalink raw reply
* Re: TCP_USER_TIMEOUT, SYN-SENT and tcp_syn_retries
From: Eric Dumazet @ 2019-09-26 18:03 UTC (permalink / raw)
To: Marek Majkowski, netdev
In-Reply-To: <74175c7e-21f3-1f68-96d4-149fe968e90f@gmail.com>
On 9/26/19 9:57 AM, Eric Dumazet wrote:
>
>
> On 9/26/19 9:46 AM, Eric Dumazet wrote:
>>
>>
>> On 9/26/19 8:05 AM, Eric Dumazet wrote:
>>>
>>>
>>> On 9/25/19 1:46 AM, Marek Majkowski wrote:
>>>> Hello my favorite mailing list!
>>>>
>>>> Recently I've been looking into TCP_USER_TIMEOUT and noticed some
>>>> strange behaviour on fresh sockets in SYN-SENT state. Full writeup:
>>>> https://blog.cloudflare.com/when-tcp-sockets-refuse-to-die/
>>>>
>>>> Here's a reproducer. It does a simple thing: sets TCP_USER_TIMEOUT and
>>>> does connect() to a blackholed IP:
>>>>
>>>> $ wget https://gist.githubusercontent.com/majek/b4ad53c5795b226d62fad1fa4a87151a/raw/cbb928cb99cd6c5aa9f73ba2d3bc0aef22fbc2bf/user-timeout-and-syn.py
>>>>
>>>> $ sudo python3 user-timeout-and-syn.py
>>>> 00:00.000000 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:01.007053 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:03.023051 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:05.007096 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:05.015037 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:05.023020 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>> 00:05.034983 IP 192.1.1.1.52974 > 244.0.0.1.1234: Flags [S]
>>>>
>>>> The connect() times out with ETIMEDOUT after 5 seconds - as intended.
>>>> But Linux (5.3.0-rc3) does something weird on the network - it sends
>>>> remaining tcp_syn_retries packets aligned to the 5s mark.
>>>>
>>>> In other words: with TCP_USER_TIMEOUT we are sending spurious SYN
>>>> packets on a timeout.
>>>>
>>>> For the record, the man page doesn't define what TCP_USER_TIMEOUT does
>>>> on SYN-SENT state.
>>>>
>>>
>>> Exactly, so far this option has only be used on established flows.
>>>
>>> Feel free to send patches if you need to override the stack behavior
>>> for connection establishment (Same remark for passive side...)
>>
>> Also please take a look at TCP_SYNCNT, which predates TCP_USER_TIMEOUT
>>
>>
>
> I will test the following :
>
> diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
> index dbd9d2d0ee63aa46ad2dda417da6ec9409442b77..1182e51a6b794d75beb8c130354d7804fc83a307 100644
> --- a/net/ipv4/tcp_timer.c
> +++ b/net/ipv4/tcp_timer.c
> @@ -220,7 +220,6 @@ static int tcp_write_timeout(struct sock *sk)
> sk_rethink_txhash(sk);
> }
> retry_until = icsk->icsk_syn_retries ? : net->ipv4.sysctl_tcp_syn_retries;
> - expired = icsk->icsk_retransmits >= retry_until;
> } else {
> if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1, 0)) {
> /* Black hole detection */
> @@ -242,9 +241,9 @@ static int tcp_write_timeout(struct sock *sk)
> if (tcp_out_of_resources(sk, do_reset))
> return 1;
> }
> - expired = retransmits_timed_out(sk, retry_until,
> - icsk->icsk_user_timeout);
> }
> + expired = retransmits_timed_out(sk, retry_until,
> + icsk->icsk_user_timeout);
> tcp_fastopen_active_detect_blackhole(sk, expired);
>
> if (BPF_SOCK_OPS_TEST_FLAG(tp, BPF_SOCK_OPS_RTO_CB_FLAG))
>
The patch works well, but reading again the man page, I see the existing behavior as
been clearly documented.
If we change the behavior, we might break applications that were setting TCP_USER_TIMEOUT
on the listener, expecting the value to b inherited to children at accept() time
but not expecting to change SYNACK rtx behavior.
On the other hand, John Maxell patch (tcp: Add tcp_clamp_rto_to_user_timeout() helper to improve accuracy)
has added this weird effect of sending remaining SYN every jiffie
remaining = icsk->icsk_user_timeout - elapsed;
if (remaining <= 0)
return 1; /* user timeout has passed; fire ASAP */
So we probably just should extend TCP_USER_TIMEOUT to SYN_SENT/SYN_RECV states
and change the man page accordingly.
TCP_USER_TIMEOUT (since Linux 2.6.37)
This option takes an unsigned int as an argument. When the value is
greater than 0, it specifies the maximum amount of time in millisec‐
onds that transmitted data may remain unacknowledged before TCP will
forcibly close the corresponding connection and return ETIMEDOUT to
the application. If the option value is specified as 0, TCP will to
use the system default.
Increasing user timeouts allows a TCP connection to survive extended
periods without end-to-end connectivity. Decreasing user timeouts
allows applications to "fail fast", if so desired. Otherwise, fail‐
ure may take up to 20 minutes with the current system defaults in a
normal WAN environment.
This option can be set during any state of a TCP connection, but is
effective only during the synchronized states of a connection (ESTAB‐
LISHED, FIN-WAIT-1, FIN-WAIT-2, CLOSE-WAIT, CLOSING, and LAST-ACK).
Moreover, when used with the TCP keepalive (SO_KEEPALIVE) option,
TCP_USER_TIMEOUT will override keepalive to determine when to close a
connection due to keepalive failure.
The option has no effect on when TCP retransmits a packet, nor when a
keepalive probe is sent.
This option, like many others, will be inherited by the socket re‐
turned by accept(2), if it was set on the listening socket.
Further details on the user timeout feature can be found in RFC 793
and RFC 5482 ("TCP User Timeout Option").
^ permalink raw reply
* Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
From: Jason Gunthorpe @ 2019-09-26 18:04 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: Jeff Kirsher, dledford@redhat.com, Mustafa Ismail,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Shiraz Saleem
In-Reply-To: <20190926180215.GA1733924@kroah.com>
On Thu, Sep 26, 2019 at 08:02:15PM +0200, gregkh@linuxfoundation.org wrote:
> On Thu, Sep 26, 2019 at 04:55:12PM +0000, Jason Gunthorpe wrote:
> > On Thu, Sep 26, 2019 at 09:45:03AM -0700, Jeff Kirsher wrote:
> > > +int i40iw_probe(struct platform_device *pdev)
> > > +{
> > > + struct i40e_peer_dev_platform_data *pdata =
> > > + dev_get_platdata(&pdev->dev);
> > > + struct i40e_info *ldev;
> >
> > I thought Greg already said not to use platform_device for this?
>
> Yes I did, which is what I thought this whole "use MFD" was supposed to
> solve. Why is a platform device still being used here?
Looks like when mfd creates the 'multi' devices it creates them as
platform_devices
/*
* Given a platform device that's been created by mfd_add_devices(), fetch
* the mfd_cell that created it.
*/
static inline const struct mfd_cell *mfd_get_cell(struct platform_device *pdev)
Jason
^ permalink raw reply
* Re: [RFC 01/20] ice: Initialize and register multi-function device to provide RDMA
From: Greg KH @ 2019-09-26 18:05 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: dledford, jgg, Tony Nguyen, netdev, linux-rdma, Dave Ertman
In-Reply-To: <20190926164519.10471-2-jeffrey.t.kirsher@intel.com>
On Thu, Sep 26, 2019 at 09:45:00AM -0700, Jeff Kirsher wrote:
> From: Tony Nguyen <anthony.l.nguyen@intel.com>
>
> The RDMA block does not advertise on the PCI bus or any other bus.
Huh? How do you "know" where it is then? Isn't is usually assigned to
a PCI device?
> Thus the ice driver needs to provide access to the RDMA hardware block
> via a virtual bus; utilize a multi-function device to provide this access.
>
> This patch initializes the driver to support RDMA as well as creates
> and registers a multi-function device for the RDMA driver to register to.
> At this point the driver is fully initialized to register a platform
> driver, however, can not yet register as the ops have not been
> implemented.
>
> We refer to the interaction of this platform device as Inter-Driver
> Communication (IDC); where the platform device is referred to as the peer
> device and the platform driver is referred to as the peer driver.
Again, no platform devices, unless it REALLY IS a platform device (i.e.
you are using device tree or platform data to find it.) Is that what
you are doing here?
confused,
greg k-h
^ permalink raw reply
* Re: [PATCH net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Florian Westphal @ 2019-09-26 18:06 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev, steffen.klassert, paulb, vladbu
In-Reply-To: <76c10ba7-5fc8-e9e8-769f-fc4d5cada7a2@gmail.com>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > +static inline void skb_ext_reset(struct sk_buff *skb)
> > +{
> > + if (skb->active_extensions) {
>
> This deserves an unlikely(skb->active_extensions) hint here ?
unlikely() isn't used in the other helpers (e.g. skb_ext_{put,del,copy}
either, should I add it there too?
^ permalink raw reply
* RE: [RFC 04/20] RDMA/irdma: Add driver framework definitions
From: Saleem, Shiraz @ 2019-09-26 18:10 UTC (permalink / raw)
To: Jason Gunthorpe, gregkh@linuxfoundation.org
Cc: Kirsher, Jeffrey T, dledford@redhat.com, Ismail, Mustafa,
netdev@vger.kernel.org, linux-rdma@vger.kernel.org
In-Reply-To: <20190926180416.GI19509@mellanox.com>
> Subject: Re: [RFC 04/20] RDMA/irdma: Add driver framework definitions
>
> On Thu, Sep 26, 2019 at 08:02:15PM +0200, gregkh@linuxfoundation.org wrote:
> > On Thu, Sep 26, 2019 at 04:55:12PM +0000, Jason Gunthorpe wrote:
> > > On Thu, Sep 26, 2019 at 09:45:03AM -0700, Jeff Kirsher wrote:
> > > > +int i40iw_probe(struct platform_device *pdev) {
> > > > + struct i40e_peer_dev_platform_data *pdata =
> > > > + dev_get_platdata(&pdev->dev);
> > > > + struct i40e_info *ldev;
> > >
> > > I thought Greg already said not to use platform_device for this?
> >
> > Yes I did, which is what I thought this whole "use MFD" was supposed
> > to solve. Why is a platform device still being used here?
>
> Looks like when mfd creates the 'multi' devices it creates them as
> platform_devices
>
> /*
> * Given a platform device that's been created by mfd_add_devices(), fetch
> * the mfd_cell that created it.
> */
> static inline const struct mfd_cell *mfd_get_cell(struct platform_device *pdev)
>
> Jason
That's right. We used the MFD framework. mfd_add_devices() registers the child devices
as platform devs. And the function drivers probe() will get a platform dev.
Shiraz
^ permalink raw reply
* [net-next v3 0/7] new PTP ioctl fixes
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller
This series contains patches to fix the various drivers which implemented
external pin input/output support. The drivers did not explicitly reject
unknown/unsupported flags.
Changes since v2:
* Split the external timestamp changes to separate patches per-driver
* Change the check for external timestamp flags to always accept all three
current flags.
* Add cc for authors of the PTP support, hopefully receiving feedback
Jacob Keller (7):
ptp: correctly disable flags on old ioctls
net: reject PTP periodic output requests with unsupported flags
mv88e6xxx: reject unsupported external timestamp flags
dp83640: reject unsupported external timestamp flags
igb: reject unsupported external timestamp flags
mlx5: reject unsupported external timestamp flags
renesas: reject unsupported external timestamp flags
drivers/net/dsa/mv88e6xxx/ptp.c | 6 +++++
drivers/net/ethernet/broadcom/tg3.c | 4 ++++
drivers/net/ethernet/intel/igb/igb_ptp.c | 10 +++++++++
.../ethernet/mellanox/mlx5/core/lib/clock.c | 10 +++++++++
drivers/net/ethernet/microchip/lan743x_ptp.c | 4 ++++
drivers/net/ethernet/renesas/ravb_ptp.c | 10 +++++++++
.../net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++++
drivers/net/phy/dp83640.c | 8 +++++++
drivers/ptp/ptp_chardev.c | 4 ++--
include/uapi/linux/ptp_clock.h | 22 +++++++++++++++++++
10 files changed, 80 insertions(+), 2 deletions(-)
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply
* [net-next v3 1/7] ptp: correctly disable flags on old ioctls
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
Felipe Balbi, David S . Miller, Christopher Hall
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Commit 415606588c61 ("PTP: introduce new versions of IOCTLs",
2019-09-13) introduced new versions of the PTP ioctls which actually
validate that the flags are acceptable values.
As part of this, it cleared the flags value using a bitwise
and+negation, in an attempt to prevent the old ioctl from accidentally
enabling new features.
This is incorrect for a couple of reasons. First, it results in
accidentally preventing previously working flags on the request ioctl.
By clearing the "valid" flags, we now no longer allow setting the
enable, rising edge, or falling edge flags.
Second, if we add new additional flags in the future, they must not be
set by the old ioctl. (Since the flag wasn't checked before, we could
potentially break userspace programs which sent garbage flag data.
The correct way to resolve this is to check for and clear all but the
originally valid flags.
Create defines indicating which flags are correctly checked and
interpreted by the original ioctls. Use these to clear any bits which
will not be correctly interpreted by the original ioctls.
In the future, new flags must be added to the VALID_FLAGS macros, but
*not* to the V1_VALID_FLAGS macros. In this way, new features may be
exposed over the v2 ioctls, but without breaking previous userspace
which happened to not clear the flags value properly. The old ioctl will
continue to behave the same way, while the new ioctl gains the benefit
of using the flags fields.
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/ptp/ptp_chardev.c | 4 ++--
include/uapi/linux/ptp_clock.h | 22 ++++++++++++++++++++++
2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 9c18476d8d10..67d0199840fd 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -155,7 +155,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EINVAL;
break;
} else if (cmd == PTP_EXTTS_REQUEST) {
- req.extts.flags &= ~PTP_EXTTS_VALID_FLAGS;
+ req.extts.flags &= PTP_EXTTS_V1_VALID_FLAGS;
req.extts.rsv[0] = 0;
req.extts.rsv[1] = 0;
}
@@ -184,7 +184,7 @@ long ptp_ioctl(struct posix_clock *pc, unsigned int cmd, unsigned long arg)
err = -EINVAL;
break;
} else if (cmd == PTP_PEROUT_REQUEST) {
- req.perout.flags &= ~PTP_PEROUT_VALID_FLAGS;
+ req.perout.flags &= PTP_PEROUT_V1_VALID_FLAGS;
req.perout.rsv[0] = 0;
req.perout.rsv[1] = 0;
req.perout.rsv[2] = 0;
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index f16301015949..59e89a1bc3bb 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -31,15 +31,37 @@
#define PTP_ENABLE_FEATURE (1<<0)
#define PTP_RISING_EDGE (1<<1)
#define PTP_FALLING_EDGE (1<<2)
+
+/*
+ * flag fields valid for the new PTP_EXTTS_REQUEST2 ioctl.
+ */
#define PTP_EXTTS_VALID_FLAGS (PTP_ENABLE_FEATURE | \
PTP_RISING_EDGE | \
PTP_FALLING_EDGE)
+/*
+ * flag fields valid for the original PTP_EXTTS_REQUEST ioctl.
+ * DO NOT ADD NEW FLAGS HERE.
+ */
+#define PTP_EXTTS_V1_VALID_FLAGS (PTP_ENABLE_FEATURE | \
+ PTP_RISING_EDGE | \
+ PTP_FALLING_EDGE)
+
/*
* Bits of the ptp_perout_request.flags field:
*/
#define PTP_PEROUT_ONE_SHOT (1<<0)
+
+/*
+ * flag fields valid for the new PTP_PEROUT_REQUEST2 ioctl.
+ */
#define PTP_PEROUT_VALID_FLAGS (PTP_PEROUT_ONE_SHOT)
+
+/*
+ * No flags are valid for the original PTP_PEROUT_REQUEST ioctl
+ */
+#define PTP_PEROUT_V1_VALID_FLAGS (0)
+
/*
* struct ptp_clock_time - represents a time value
*
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 2/7] net: reject PTP periodic output requests with unsupported flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Richard Cochran,
Felipe Balbi, David S . Miller, Christopher Hall
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Commit 823eb2a3c4c7 ("PTP: add support for one-shot output") introduced
a new flag for the PTP periodic output request ioctl. This flag is not
currently supported by any driver.
Fix all drivers which implement the periodic output request ioctl to
explicitly reject any request with flags they do not understand. This
ensures that the driver does not accidentally misinterpret the
PTP_PEROUT_ONE_SHOT flag, or any new flag introduced in the future.
This is important for forward compatibility: if a new flag is
introduced, the driver should reject requests to enable the flag until
the driver has actually been modified to support the flag in question.
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: David S. Miller <davem@davemloft.net>
Cc: Christopher Hall <christopher.s.hall@intel.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/broadcom/tg3.c | 4 ++++
drivers/net/ethernet/intel/igb/igb_ptp.c | 4 ++++
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 4 ++++
drivers/net/ethernet/microchip/lan743x_ptp.c | 4 ++++
drivers/net/ethernet/renesas/ravb_ptp.c | 4 ++++
drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c | 4 ++++
drivers/net/phy/dp83640.c | 3 +++
7 files changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index 77f3511b97de..ca3aa1250dd1 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6280,6 +6280,10 @@ static int tg3_ptp_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (rq->perout.index != 0)
return -EINVAL;
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index fd3071f55bd3..4997963149f6 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -551,6 +551,10 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_PEROUT,
rq->perout.index);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index 0059b290e095..cff6b60de304 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -290,6 +290,10 @@ static int mlx5_perout_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP;
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
if (rq->perout.index >= clock->ptp_info.n_pins)
return -EINVAL;
diff --git a/drivers/net/ethernet/microchip/lan743x_ptp.c b/drivers/net/ethernet/microchip/lan743x_ptp.c
index 57b26c2acf87..e8fe9a90fe4f 100644
--- a/drivers/net/ethernet/microchip/lan743x_ptp.c
+++ b/drivers/net/ethernet/microchip/lan743x_ptp.c
@@ -429,6 +429,10 @@ static int lan743x_ptp_perout(struct lan743x_adapter *adapter, int on,
int pulse_width = 0;
int perout_bit = 0;
+ /* Reject requests with unsupported flags */
+ if (perout->flags)
+ return -EOPNOTSUPP;
+
if (!on) {
lan743x_ptp_perout_off(adapter);
return 0;
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 9a42580693cb..638f1fc2166f 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -211,6 +211,10 @@ static int ravb_ptp_perout(struct ptp_clock_info *ptp,
unsigned long flags;
int error = 0;
+ /* Reject requests with unsupported flags */
+ if (req->flags)
+ return -EOPNOTSUPP;
+
if (req->index)
return -EINVAL;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
index 173493db038c..352dc4c68625 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.c
@@ -140,6 +140,10 @@ static int stmmac_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
+
cfg = &priv->pps[rq->perout.index];
cfg->start.tv_sec = rq->perout.start.sec;
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 6580094161a9..04ad77758920 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -491,6 +491,9 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
return 0;
case PTP_CLK_REQ_PEROUT:
+ /* Reject requests with unsupported flags */
+ if (rq->perout.flags)
+ return -EOPNOTSUPP;
if (rq->perout.index >= N_PER_OUT)
return -EINVAL;
return periodic_output(clock, rq, on, rq->perout.index);
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 3/7] mv88e6xxx: reject unsupported external timestamp flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Brandon Streiff
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Fix the mv88e6xxx PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.
In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.
Cc: Brandon Streiff <brandon.streiff@ni.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/dsa/mv88e6xxx/ptp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/ptp.c b/drivers/net/dsa/mv88e6xxx/ptp.c
index 073cbd0bb91b..076e622a64d6 100644
--- a/drivers/net/dsa/mv88e6xxx/ptp.c
+++ b/drivers/net/dsa/mv88e6xxx/ptp.c
@@ -273,6 +273,12 @@ static int mv88e6352_ptp_enable_extts(struct mv88e6xxx_chip *chip,
int pin;
int err;
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
+
pin = ptp_find_pin(chip->ptp_clock, PTP_PF_EXTTS, rq->extts.index);
if (pin < 0)
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 4/7] dp83640: reject unsupported external timestamp flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller,
Stefan Sørensen, Richard Cochran
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Fix the dp83640 PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.
In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.
Cc: Stefan Sørensen <stefan.sorensen@spectralink.com>
Cc: Richard Cochran <richardcochran@gmail.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/phy/dp83640.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 04ad77758920..2781b0e2d947 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -469,6 +469,11 @@ static int ptp_dp83640_enable(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
index = rq->extts.index;
if (index >= N_EXT_TS)
return -EINVAL;
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 5/7] igb: reject unsupported external timestamp flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Fix the igb PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.
In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/intel/igb/igb_ptp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
index 4997963149f6..0bce3e0f1af0 100644
--- a/drivers/net/ethernet/intel/igb/igb_ptp.c
+++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
@@ -521,6 +521,12 @@ static int igb_ptp_feature_enable_i210(struct ptp_clock_info *ptp,
switch (rq->type) {
case PTP_CLK_REQ_EXTTS:
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
+
if (on) {
pin = ptp_find_pin(igb->ptp_clock, PTP_PF_EXTTS,
rq->extts.index);
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 6/7] mlx5: reject unsupported external timestamp flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev
Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Feras Daoud,
Eugenia Emantayev
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Fix the mlx5 core PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.
In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.
Cc: Feras Daoud <ferasda@mellanox.com>
Cc: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
index cff6b60de304..9a40f24e3193 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/clock.c
@@ -236,6 +236,12 @@ static int mlx5_extts_configure(struct ptp_clock_info *ptp,
if (!MLX5_PPS_CAP(mdev))
return -EOPNOTSUPP;
+ /* Reject requests with unsupported flags */
+ if (rq->extts.flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
+
if (rq->extts.index >= clock->ptp_info.n_pins)
return -EINVAL;
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* [net-next v3 7/7] renesas: reject unsupported external timestamp flags
From: Jacob Keller @ 2019-09-26 18:11 UTC (permalink / raw)
To: netdev; +Cc: Intel Wired LAN, Jeffrey Kirsher, Jacob Keller, Sergei Shtylyov
In-Reply-To: <20190926181109.4871-1-jacob.e.keller@intel.com>
Fix the renesas PTP support to explicitly reject any future flags that
get added to the external timestamp request ioctl.
In order to maintain currently functioning code, this patch accepts all
three current flags. This is because the PTP_RISING_EDGE and
PTP_FALLING_EDGE flags have unclear semantics and each driver seems to
have interpreted them slightly differently.
Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
drivers/net/ethernet/renesas/ravb_ptp.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 638f1fc2166f..666dbee48097 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -182,6 +182,12 @@ static int ravb_ptp_extts(struct ptp_clock_info *ptp,
struct net_device *ndev = priv->ndev;
unsigned long flags;
+ /* Reject requests with unsupported flags */
+ if (req->flags & ~(PTP_ENABLE_FEATURE |
+ PTP_RISING_EDGE |
+ PTP_FALLING_EDGE))
+ return -EOPNOTSUPP;
+
if (req->index)
return -EINVAL;
--
2.23.0.245.gf157bbb9169d
^ permalink raw reply related
* Re: rtlwifi: Remove excessive check in _rtl_ps_inactive_ps()
From: Markus Elfring @ 2019-09-26 18:11 UTC (permalink / raw)
To: Denis Efremov, Kalle Valo, Ping-Ke Shih, David S. Miller,
linux-wireless, netdev
Cc: linux-kernel, kernel-janitors, Larry Finger
In-Reply-To: <20190925205858.30216-1-efremov@linux.com>
> +++ b/drivers/net/wireless/realtek/rtlwifi/ps.c
> @@ -161,8 +161,7 @@ static void _rtl_ps_inactive_ps(struct ieee80211_hw *hw)
> if (ppsc->inactive_pwrstate == ERFON &&
> rtlhal->interface == INTF_PCI) {
> if ((ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM) &&
Can it make sense to reduce the nesting level for these condition checks
besides the suggested deletion of duplicate source code?
Regards,
Markus
^ permalink raw reply
* Re: Are BPF tail calls only supposed to work with pinned maps?
From: Daniel Borkmann @ 2019-09-26 18:14 UTC (permalink / raw)
To: Toke Høiland-Jørgensen; +Cc: netdev, bpf
In-Reply-To: <87zhir19s1.fsf@toke.dk>
On Thu, Sep 26, 2019 at 03:12:30PM +0200, Toke Høiland-Jørgensen wrote:
> Daniel Borkmann <daniel@iogearbox.net> writes:
> > On Thu, Sep 26, 2019 at 01:23:38PM +0200, Toke Høiland-Jørgensen wrote:
> > [...]
> >> While working on a prototype of the XDP chain call feature, I ran into
> >> some strange behaviour with tail calls: If I create a userspace program
> >> that loads two XDP programs, one of which tail calls the other, the tail
> >> call map would appear to be empty even though the userspace program
> >> populates it as part of the program loading.
> >>
> >> I eventually tracked this down to this commit:
> >> c9da161c6517 ("bpf: fix clearing on persistent program array maps")
> >
> > Correct.
> >
> >> Which clears PROG_ARRAY maps whenever the last uref to it disappears
> >> (which it does when my loader exits after attaching the XDP program).
> >>
> >> This effectively means that tail calls only work if the PROG_ARRAY map
> >> is pinned (or the process creating it keeps running). And as far as I
> >> can tell, the inner_map reference in bpf_map_fd_get_ptr() doesn't bump
> >> the uref either, so presumably if one were to create a map-in-map
> >> construct with tail call pointer in the inner map(s), each inner map
> >> would also need to be pinned (haven't tested this case)?
> >
> > There is no map in map support for tail calls today.
>
> Not directly, but can't a program do:
>
> tail_call_map = bpf_map_lookup(outer_map, key);
> bpf_tail_call(tail_call_map, idx);
Nope, that is what I meant, bpf_map_meta_alloc() will bail out in that
case.
> >> Is this really how things are supposed to work? From an XDP use case PoV
> >> this seems somewhat surprising...
> >>
> >> Or am I missing something obvious here?
> >
> > The way it was done like this back then was in order to break up cyclic
> > dependencies as otherwise the programs and maps involved would never get
> > freed as they reference themselves and live on in the kernel forever
> > consuming potentially large amount of resources, so orchestration tools
> > like Cilium typically just pin the maps in bpf fs (like most other maps
> > it uses and accesses from agent side) in order to up/downgrade the agent
> > while keeping BPF datapath intact.
>
> Right. I can see how the cyclic reference thing gets thorny otherwise.
> However, the behaviour was somewhat surprising to me; is it documented
> anywhere?
Haven't updated the BPF guide in a while [0], I don't think I documented
this detail back then, so right now only in the git log. Improvements to
the reference guide definitely welcome.
Thanks,
Daniel
[0] https://cilium.readthedocs.io/en/latest/bpf/
https://github.com/cilium/cilium/blob/master/Documentation/bpf.rst
^ permalink raw reply
* Re: [PATCH net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Eric Dumazet @ 2019-09-26 18:15 UTC (permalink / raw)
To: Florian Westphal, Eric Dumazet; +Cc: netdev, steffen.klassert, paulb, vladbu
In-Reply-To: <20190926180609.GB9938@breakpoint.cc>
On 9/26/19 11:06 AM, Florian Westphal wrote:
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>> +static inline void skb_ext_reset(struct sk_buff *skb)
>>> +{
>>> + if (skb->active_extensions) {
>>
>> This deserves an unlikely(skb->active_extensions) hint here ?
>
> unlikely() isn't used in the other helpers (e.g. skb_ext_{put,del,copy}
> either, should I add it there too?
>
At least in GRO paths unlikely() makes sense.
For other paths it might not be the case.
^ permalink raw reply
* Re: [PATCH iproute2(-next) v2 1/1] ip: fix ip route show json output for multipath nexthops
From: Julien Fortin @ 2019-09-26 18:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, Roopa Prabhu, dsahern
In-Reply-To: <20190926090755.78b6234e@hermes.lan>
On Thu, Sep 26, 2019 at 6:07 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 26 Sep 2019 17:29:34 +0200
> Julien Fortin <julien@cumulusnetworks.com> wrote:
>
> > + print_string(PRINT_ANY, "dev",
> > + "%s", ll_index_to_name(nh->rtnh_ifindex))
>
> you might want to use interface color for this?
Since this is not part of the existing code, I guess this can be added
in a later patch.
^ permalink raw reply
* Re: [PATCH] bpf: clean up indentation issue
From: Daniel Borkmann @ 2019-09-26 18:16 UTC (permalink / raw)
To: Colin King
Cc: Alexei Starovoitov, Martin KaFai Lau, Song Liu, Yonghong Song,
netdev, bpf, kernel-janitors, linux-kernel
In-Reply-To: <20190925093835.19515-1-colin.king@canonical.com>
On Wed, Sep 25, 2019 at 10:38:35AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is a statement that is indented one level too deeply,
> remove the extraneous tab.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* [PATCH] vsock/virtio: add support for MSG_PEEK
From: Matias Ezequiel Vara Larsen @ 2019-09-26 18:23 UTC (permalink / raw)
To: stefanha
Cc: davem, kvm, virtualization, netdev, linux-kernel, matiasevara,
sgarzare
This patch adds support for MSG_PEEK. In such a case, packets are not
removed from the rx_queue and credit updates are not sent.
Signed-off-by: Matias Ezequiel Vara Larsen <matiasevara@gmail.com>
---
net/vmw_vsock/virtio_transport_common.c | 50 +++++++++++++++++++++++++++++++--
1 file changed, 47 insertions(+), 3 deletions(-)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 94cc0fa..938f2ed 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -264,6 +264,50 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
}
static ssize_t
+virtio_transport_stream_do_peek(struct vsock_sock *vsk,
+ struct msghdr *msg,
+ size_t len)
+{
+ struct virtio_vsock_sock *vvs = vsk->trans;
+ struct virtio_vsock_pkt *pkt;
+ size_t bytes, total = 0;
+ int err = -EFAULT;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ list_for_each_entry(pkt, &vvs->rx_queue, list) {
+ if (total == len)
+ break;
+
+ bytes = len - total;
+ if (bytes > pkt->len - pkt->off)
+ bytes = pkt->len - pkt->off;
+
+ /* sk_lock is held by caller so no one else can dequeue.
+ * Unlock rx_lock since memcpy_to_msg() may sleep.
+ */
+ spin_unlock_bh(&vvs->rx_lock);
+
+ err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
+ if (err)
+ goto out;
+
+ spin_lock_bh(&vvs->rx_lock);
+
+ total += bytes;
+ }
+
+ spin_unlock_bh(&vvs->rx_lock);
+
+ return total;
+
+out:
+ if (total)
+ err = total;
+ return err;
+}
+
+static ssize_t
virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
struct msghdr *msg,
size_t len)
@@ -330,9 +374,9 @@ virtio_transport_stream_dequeue(struct vsock_sock *vsk,
size_t len, int flags)
{
if (flags & MSG_PEEK)
- return -EOPNOTSUPP;
-
- return virtio_transport_stream_do_dequeue(vsk, msg, len);
+ return virtio_transport_stream_do_peek(vsk, msg, len);
+ else
+ return virtio_transport_stream_do_dequeue(vsk, msg, len);
}
EXPORT_SYMBOL_GPL(virtio_transport_stream_dequeue);
--
2.7.4
^ permalink raw reply related
* [PATCH v2 net] sk_buff: drop all skb extensions on free and skb scrubbing
From: Florian Westphal @ 2019-09-26 18:37 UTC (permalink / raw)
To: netdev; +Cc: steffen.klassert, paulb, vladbu, Florian Westphal
Now that we have a 3rd extension, add a new helper that drops the
extension space and use it when we need to scrub an sk_buff.
At this time, scrubbing clears secpath and bridge netfilter data, but
retains the tc skb extension, after this patch all three get cleared.
NAPI reuse/free assumes we can only have a secpath attached to skb, but
it seems better to clear all extensions there as well.
v2: add unlikely hint (Eric Dumazet)
Fixes: 95a7233c452a ("net: openvswitch: Set OvS recirc_id from tc chain index")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/skbuff.h | 9 +++++++++
net/core/dev.c | 4 ++--
net/core/skbuff.c | 2 +-
3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 907209c0794e..e7d3b1a513ef 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -4144,8 +4144,17 @@ static inline void *skb_ext_find(const struct sk_buff *skb, enum skb_ext_id id)
return NULL;
}
+
+static inline void skb_ext_reset(struct sk_buff *skb)
+{
+ if (unlikely(skb->active_extensions)) {
+ __skb_ext_put(skb->extensions);
+ skb->active_extensions = 0;
+ }
+}
#else
static inline void skb_ext_put(struct sk_buff *skb) {}
+static inline void skb_ext_reset(struct sk_buff *skb) {}
static inline void skb_ext_del(struct sk_buff *skb, int unused) {}
static inline void __skb_ext_copy(struct sk_buff *d, const struct sk_buff *s) {}
static inline void skb_ext_copy(struct sk_buff *dst, const struct sk_buff *s) {}
diff --git a/net/core/dev.c b/net/core/dev.c
index 71b18e80389f..bf3ed413abaf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5666,7 +5666,7 @@ EXPORT_SYMBOL(gro_find_complete_by_type);
static void napi_skb_free_stolen_head(struct sk_buff *skb)
{
skb_dst_drop(skb);
- secpath_reset(skb);
+ skb_ext_put(skb);
kmem_cache_free(skbuff_head_cache, skb);
}
@@ -5733,7 +5733,7 @@ static void napi_reuse_skb(struct napi_struct *napi, struct sk_buff *skb)
skb->encapsulation = 0;
skb_shinfo(skb)->gso_type = 0;
skb->truesize = SKB_TRUESIZE(skb_end_offset(skb));
- secpath_reset(skb);
+ skb_ext_reset(skb);
napi->skb = skb;
}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f12e8a050edb..01d65206f4fb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5119,7 +5119,7 @@ void skb_scrub_packet(struct sk_buff *skb, bool xnet)
skb->skb_iif = 0;
skb->ignore_df = 0;
skb_dst_drop(skb);
- secpath_reset(skb);
+ skb_ext_reset(skb);
nf_reset(skb);
nf_reset_trace(skb);
--
2.21.0
^ 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