Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH iproute2 0/3] do not set IPv6-only options on IPv4 addresses
From: Andrea Claudi @ 2019-06-24 21:38 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, David Ahern
In-Reply-To: <20190624102041.25224fae@hermes.lan>

On Mon, Jun 24, 2019 at 7:21 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Mon, 24 Jun 2019 19:05:52 +0200
> Andrea Claudi <aclaudi@redhat.com> wrote:
>
> > 'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but
> > it is possible to set them on IPv4 addresses, too. This should
> > not be possible.
> >
> > Fix this adding a check on the protocol family before setting
> > the flags, and exiting with invarg() on error.
> >
> > Andrea Claudi (3):
> >   ip address: do not set nodad option for IPv4 addresses
> >   ip address: do not set home option for IPv4 addresses
> >   ip address: do not set mngtmpaddr option for IPv4 addresses
> >
> >  ip/ipaddress.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
>
> Maybe this should be a warning, not a failure.
> A little concerned that there will be some user with a scripted setup
> that this breaks.

Hi Stephen,
I think that if a script wrongly uses some of these flags on a IPv4
address, it most probably operates on an unexpected address, since
everyone is aware that these flags are IPv6 only. In other words we
are breaking a scripted setup that is already broken.
In this case it's probably worth exiting with error and give the
author the chance to fix the script, otherwise the error can go
unnoticed.

If you prefer, I can send a v2 with warnings instead of errors, just
let me know.

Regards,
Andrea

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 21:30 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andy Lutomirski, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <7f36edf7-3120-975e-b643-3c0fa470bafd@iogearbox.net>

On Mon, Jun 24, 2019 at 2:22 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> Agree, for example, bpf_probe_write_user() can never write into
> kernel memory (only user one). Just thinking out loud, wouldn't it
> be cleaner and more generic to perform this check at the actual function
> which performs the kernel memory without faulting? All three of these
> are in mm/maccess.c, and the very few occasions that override the
> probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
> so this would catch all of them wrt lockdown restrictions. Otherwise
> you'd need to keep tracking every bit of new code being merged that
> calls into one of these, no? That way you only need to do it once like
> below and are guaranteed that the check catches these in future as well.

Not all paths into probe_kernel_read/write are from entry points that
need to be locked down (eg, as far as I can tell ftrace can't leak
anything interesting here).

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Daniel Borkmann @ 2019-06-24 20:59 UTC (permalink / raw)
  To: Andy Lutomirski, Matthew Garrett
  Cc: James Morris, LSM List, Linux Kernel Mailing List, Linux API,
	David Howells, Alexei Starovoitov, Network Development,
	Chun-Yi Lee, Jann Horn, bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>

On 06/24/2019 10:08 PM, Andy Lutomirski wrote:
> On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>>> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
>>>> From: David Howells <dhowells@redhat.com>
>>>>
>>>> There are some bpf functions can be used to read kernel memory:
>>>
>>> Nit: that
>>
>> Fixed.
>>
>>>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
>>>
>>> Please explain how bpf_probe_write_user reads kernel memory ... ?!
>>
>> Ha.
>>
>>>> private keys in kernel memory (e.g. the hibernation image signing key) to
>>>> be read by an eBPF program and kernel memory to be altered without
>>>
>>> ... and while we're at it, also how they allow "kernel memory to be
>>> altered without restriction". I've been pointing this false statement
>>> out long ago.
>>
>> Yup. How's the following description:
>>
>>     bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>>
>>     There are some bpf functions that can be used to read kernel memory and
>>     exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>>     bpf_trace_printk.  These could be abused to (eg) allow private
>> keys in kernel
>>     memory to be leaked. Disable them if the kernel has been locked
>> down in confidentiality
>>     mode.
> 
> I'm confused.  I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?

Agree, for example, bpf_probe_write_user() can never write into
kernel memory (only user one). Just thinking out loud, wouldn't it
be cleaner and more generic to perform this check at the actual function
which performs the kernel memory without faulting? All three of these
are in mm/maccess.c, and the very few occasions that override the
probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
so this would catch all of them wrt lockdown restrictions. Otherwise
you'd need to keep tracking every bit of new code being merged that
calls into one of these, no? That way you only need to do it once like
below and are guaranteed that the check catches these in future as well.

Thanks,
Daniel

diff --git a/mm/maccess.c b/mm/maccess.c
index 482d4d6..2c8220f 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -29,6 +29,9 @@ long __probe_kernel_read(void *dst, const void *src, size_t size)
 	long ret;
 	mm_segment_t old_fs = get_fs();

+	if (security_locked_down(LOCKDOWN_KERNEL_READ))
+		return -EFAULT;
+
 	set_fs(KERNEL_DS);
 	pagefault_disable();
 	ret = __copy_from_user_inatomic(dst,
@@ -57,6 +60,9 @@ long __probe_kernel_write(void *dst, const void *src, size_t size)
 	long ret;
 	mm_segment_t old_fs = get_fs();

+	if (security_locked_down(LOCKDOWN_KERNEL_WRITE))
+		return -EFAULT;
+
 	set_fs(KERNEL_DS);
 	pagefault_disable();
 	ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
@@ -90,6 +96,9 @@ long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count)
 	const void *src = unsafe_addr;
 	long ret;

+	if (security_locked_down(LOCKDOWN_KERNEL_READ))
+		return -EFAULT;
+
 	if (unlikely(count <= 0))
 		return 0;


^ permalink raw reply related

* [PATCH] sis900: remove TxIDLE
From: Sergej Benilov @ 2019-06-24 21:21 UTC (permalink / raw)
  To: venza, netdev; +Cc: Sergej Benilov

Before "sis900: fix TX completion" patch, TX completion was done on TxIDLE interrupt.
TX completion also was the only thing done on TxIDLE interrupt.
Since "sis900: fix TX completion", TX completion is done on TxDESC interrupt.
So it is not necessary any more to set and to check for TxIDLE.

Eliminate TxIDLE from sis900.
Correct some typos, too.

Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
---
 drivers/net/ethernet/sis/sis900.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index 9b036c857..aba6eea72 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -360,7 +360,7 @@ static int sis635_get_mac_addr(struct pci_dev *pci_dev,
  *	SiS962 or SiS963 model, use EEPROM to store MAC address. And EEPROM
  *	is shared by
  *	LAN and 1394. When access EEPROM, send EEREQ signal to hardware first
- *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be access
+ *	and wait for EEGNT. If EEGNT is ON, EEPROM is permitted to be accessed
  *	by LAN, otherwise is not. After MAC address is read from EEPROM, send
  *	EEDONE signal to refuse EEPROM access by LAN.
  *	The EEPROM map of SiS962 or SiS963 is different to SiS900.
@@ -882,7 +882,7 @@ static void mdio_reset(struct sis900_private *sp)
  *	mdio_read - read MII PHY register
  *	@net_dev: the net device to read
  *	@phy_id: the phy address to read
- *	@location: the phy regiester id to read
+ *	@location: the phy register id to read
  *
  *	Read MII registers through MDIO and MDC
  *	using MDIO management frame structure and protocol(defined by ISO/IEC).
@@ -926,7 +926,7 @@ static int mdio_read(struct net_device *net_dev, int phy_id, int location)
  *	mdio_write - write MII PHY register
  *	@net_dev: the net device to write
  *	@phy_id: the phy address to write
- *	@location: the phy regiester id to write
+ *	@location: the phy register id to write
  *	@value: the register value to write with
  *
  *	Write MII registers with @value through MDIO and MDC
@@ -1057,7 +1057,7 @@ sis900_open(struct net_device *net_dev)
 	sis900_set_mode(sis_priv, HW_SPEED_10_MBPS, FDX_CAPABLE_HALF_SELECTED);
 
 	/* Enable all known interrupts by setting the interrupt mask. */
-	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxIDLE | TxDESC);
+	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxDESC);
 	sw32(cr, RxENA | sr32(cr));
 	sw32(ier, IE);
 
@@ -1101,7 +1101,7 @@ sis900_init_rxfilter (struct net_device * net_dev)
 		sw32(rfdr, w);
 
 		if (netif_msg_hw(sis_priv)) {
-			printk(KERN_DEBUG "%s: Receive Filter Addrss[%d]=%x\n",
+			printk(KERN_DEBUG "%s: Receive Filter Address[%d]=%x\n",
 			       net_dev->name, i, sr32(rfdr));
 		}
 	}
@@ -1148,7 +1148,7 @@ sis900_init_tx_ring(struct net_device *net_dev)
  *	@net_dev: the net device to initialize for
  *
  *	Initialize the Rx descriptor ring,
- *	and pre-allocate recevie buffers (socket buffer)
+ *	and pre-allocate receive buffers (socket buffer)
  */
 
 static void
@@ -1578,7 +1578,7 @@ static void sis900_tx_timeout(struct net_device *net_dev)
 	sw32(txdp, sis_priv->tx_ring_dma);
 
 	/* Enable all known interrupts by setting the interrupt mask. */
-	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxIDLE | TxDESC);
+	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxDESC);
 }
 
 /**
@@ -1674,8 +1674,8 @@ static irqreturn_t sis900_interrupt(int irq, void *dev_instance)
 	do {
 		status = sr32(isr);
 
-		if ((status & (HIBERR|TxURN|TxERR|TxIDLE|TxDESC|RxORN|RxERR|RxOK)) == 0)
-			/* nothing intresting happened */
+		if ((status & (HIBERR|TxURN|TxERR|TxDESC|RxORN|RxERR|RxOK)) == 0)
+			/* nothing interesting happened */
 			break;
 		handled = 1;
 
@@ -1684,7 +1684,7 @@ static irqreturn_t sis900_interrupt(int irq, void *dev_instance)
 			/* Rx interrupt */
 			sis900_rx(net_dev);
 
-		if (status & (TxURN | TxERR | TxIDLE | TxDESC))
+		if (status & (TxURN | TxERR | TxDESC))
 			/* Tx interrupt */
 			sis900_finish_xmit(net_dev);
 
@@ -1897,7 +1897,7 @@ static void sis900_finish_xmit (struct net_device *net_dev)
 		if (tx_status & OWN) {
 			/* The packet is not transmitted yet (owned by hardware) !
 			 * Note: this is an almost impossible condition
-			 * in case of TxDESC ('descriptor interrupt') */
+			 * on TxDESC interrupt ('descriptor interrupt') */
 			break;
 		}
 
@@ -2473,7 +2473,7 @@ static int sis900_resume(struct pci_dev *pci_dev)
 	sis900_set_mode(sis_priv, HW_SPEED_10_MBPS, FDX_CAPABLE_HALF_SELECTED);
 
 	/* Enable all known interrupts by setting the interrupt mask. */
-	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxIDLE | TxDESC);
+	sw32(imr, RxSOVR | RxORN | RxERR | RxOK | TxURN | TxERR | TxDESC);
 	sw32(cr, RxENA | sr32(cr));
 	sw32(ier, IE);
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH bpf-next v8 1/9] bpf: implement getsockopt and setsockopt hooks
From: Andrii Nakryiko @ 2019-06-24 21:19 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: Networking, bpf, David S. Miller, Alexei Starovoitov,
	Daniel Borkmann, Martin Lau
In-Reply-To: <20190624162429.16367-2-sdf@google.com>

On Mon, Jun 24, 2019 at 1:11 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Implement new BPF_PROG_TYPE_CGROUP_SOCKOPT program type and
> BPF_CGROUP_{G,S}ETSOCKOPT cgroup hooks.
>
> BPF_CGROUP_SETSOCKOPT get a read-only view of the setsockopt arguments.
> BPF_CGROUP_GETSOCKOPT can modify the supplied buffer.
> Both of them reuse existing PTR_TO_PACKET{,_END} infrastructure.
>
> The buffer memory is pre-allocated (because I don't think there is
> a precedent for working with __user memory from bpf). This might be
> slow to do for each {s,g}etsockopt call, that's why I've added
> __cgroup_bpf_prog_array_is_empty that exits early if there is nothing
> attached to a cgroup. Note, however, that there is a race between
> __cgroup_bpf_prog_array_is_empty and BPF_PROG_RUN_ARRAY where cgroup
> program layout might have changed; this should not be a problem
> because in general there is a race between multiple calls to
> {s,g}etsocktop and user adding/removing bpf progs from a cgroup.
>
> The return code of the BPF program is handled as follows:
> * 0: EPERM
> * 1: success, continue with next BPF program in the cgroup chain
>
> v8:
> * use s32 for optlen (Andrii Nakryiko)
>
> v7:
> * return only 0 or 1 (Alexei Starovoitov)
> * always run all progs (Alexei Starovoitov)
> * use optval=0 as kernel bypass in setsockopt (Alexei Starovoitov)
>   (decided to use optval=-1 instead, optval=0 might be a valid input)
> * call getsockopt hook after kernel handlers (Alexei Starovoitov)
>
> v6:
> * rework cgroup chaining; stop as soon as bpf program returns
>   0 or 2; see patch with the documentation for the details
> * drop Andrii's and Martin's Acked-by (not sure they are comfortable
>   with the new state of things)
>
> v5:
> * skip copy_to_user() and put_user() when ret == 0 (Martin Lau)
>
> v4:
> * don't export bpf_sk_fullsock helper (Martin Lau)
> * size != sizeof(__u64) for uapi pointers (Martin Lau)
> * offsetof instead of bpf_ctx_range when checking ctx access (Martin Lau)
>
> v3:
> * typos in BPF_PROG_CGROUP_SOCKOPT_RUN_ARRAY comments (Andrii Nakryiko)
> * reverse christmas tree in BPF_PROG_CGROUP_SOCKOPT_RUN_ARRAY (Andrii
>   Nakryiko)
> * use __bpf_md_ptr instead of __u32 for optval{,_end} (Martin Lau)
> * use BPF_FIELD_SIZEOF() for consistency (Martin Lau)
> * new CG_SOCKOPT_ACCESS macro to wrap repeated parts
>
> v2:
> * moved bpf_sockopt_kern fields around to remove a hole (Martin Lau)
> * aligned bpf_sockopt_kern->buf to 8 bytes (Martin Lau)
> * bpf_prog_array_is_empty instead of bpf_prog_array_length (Martin Lau)
> * added [0,2] return code check to verifier (Martin Lau)
> * dropped unused buf[64] from the stack (Martin Lau)
> * use PTR_TO_SOCKET for bpf_sockopt->sk (Martin Lau)
> * dropped bpf_target_off from ctx rewrites (Martin Lau)
> * use return code for kernel bypass (Martin Lau & Andrii Nakryiko)
>
> Cc: Martin Lau <kafai@fb.com>
> Signed-off-by: Stanislav Fomichev <sdf@google.com>
> ---

Acked-by: Andrii Nakryiko <andriin@fb.com>

^ permalink raw reply

* Re: [PATCH] FDDI: defza: Include linux/io-64-nonatomic-lo-hi.h
From: Paul Burton @ 2019-06-24 21:16 UTC (permalink / raw)
  To: Paul Burton
  Cc: Maciej W. Rozycki, David S. Miller, netdev@vger.kernel.org,
	Paul Burton, Serge Semin, linux-mips@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-mips@vger.kernel.org
In-Reply-To: <20190620221224.27352-1-paul.burton@mips.com>

Hello,

Paul Burton wrote:
> Currently arch/mips/include/asm/io.h provides 64b memory accessor
> functions such as readq & writeq even on MIPS32 platforms where those
> accessors cannot actually perform a 64b memory access. They instead
> BUG(). This is unfortunate for drivers which either #ifdef on the
> presence of these accessors, or can function with non-atomic
> implementations of them found in either linux/io-64-nonatomic-lo-hi.h or
> linux/io-64-nonatomic-hi-lo.h. As such we're preparing to remove the
> definitions of these 64b accessor functions for MIPS32 kernels.
> 
> In preparation for this, include linux/io-64-nonatomic-lo-hi.h in
> defza.c in order to provide a non-atomic implementation of the
> readq_relaxed & writeq_relaxed functions that are used by this code. In
> practice this will have no runtime effect, since use of the 64b accessor
> functions is conditional upon sizeof(unsigned long) == 8, ie. upon
> CONFIG_64BIT=y. This means the calls to these non-atomic readq & writeq
> implementations will be optimized out anyway, but we need their
> definitions to keep the compiler happy.
> 
> For 64bit kernels using this code this change should also have no effect
> because asm/io.h will continue to provide the definitions of
> readq_relaxed & writeq_relaxed, which linux/io-64-nonatomic-lo-hi.h
> checks for before defining itself.
> 
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Cc: Serge Semin <Sergey.Semin@t-platforms.ru>
> Cc: "Maciej W. Rozycki" <macro@linux-mips.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-mips@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Maciej W. Rozycki <macro@linux-mips.org>
> Acked-by: David S. Miller <davem@davemloft.net>

Applied to mips-next.

Thanks,
    Paul

[ This message was auto-generated; if you believe anything is incorrect
  then please email paul.burton@mips.com to report it. ]

^ permalink raw reply

* Re: WWAN Controller Framework (was IPA [PATCH v2 00/17])
From: Alex Elder @ 2019-06-24 21:16 UTC (permalink / raw)
  To: Dan Williams, davem, arnd, bjorn.andersson, ilias.apalodimas
  Cc: evgreen, benchan, ejcaruso, cpratapa, syadagir, subashab,
	abhishek.esse, netdev, devicetree, linux-kernel, linux-soc,
	linux-arm-kernel, linux-arm-msm
In-Reply-To: <f0fcee096d779837abc46e7badae9105ee8aaecf.camel@redhat.com>

On 6/24/19 2:54 PM, Dan Williams wrote:
> On Mon, 2019-06-24 at 11:30 -0500, Alex Elder wrote:
>> OK I want to try to organize a little more concisely some of the
>> discussion on this, because there is a very large amount of volume
>> to date and I think we need to try to narrow the focus back down
>> again.
>>
>> I'm going to use a few terms here.  Some of these I really don't
>> like, but I want to be unambiguous *and* (at least for now) I want
>> to avoid the very overloaded term "device".
>>
>> I have lots more to say, but let's start with a top-level picture,
>> to make sure we're all on the same page.
>>
>>          WWAN Communication
>>          Channel (Physical)
>>                  |     ------------------------
>> ------------     v     |           :+ Control |  \
>>>          |-----------|           :+ Data    |  |
>>>    AP    |           | WWAN unit :+ Voice   |   > Functions
>>>          |===========|           :+ GPS     |  |
>> ------------     ^     |           :+ ...     |  /
>>                  |     -------------------------
>>           Multiplexed WWAN
>>            Communication
>>          Channel (Physical)
>>
>> - The *AP* is the main CPU complex that's running Linux on one or
>>   more CPU cores.
>> - A *WWAN unit* is an entity that shares one or more physical
>>   *WWAN communication channels* with the AP.
> 
> You could just say "WWAN modem" here.

That sounds great to me.

>> - A *WWAN communication channel* is a bidirectional means of
>>   carrying data between the AP and WWAN unit.
>> - A WWAN communication channel carries data using a *WWAN protocol*.
>> - A WWAN unit implements one or more *WWAN functions*, such as
>>   5G data, LTE voice, GPS, and so on.
> 
> Go more generic here. Not just 5G data but any WWAN IP-based data
> (GPRS, EDGE, CDMA, UMTS, EVDO, LTE, 5G, etc). And not just LTE voice
> but any voice data; plenty of devices don't support LTE but still have
> "WWAN logical communication channels"

I really meant *any* sort of function, and was only trying
to give a few examples.  So yes, my meaning was completely
generic, as you suggest.

>> - A WWAN unit shall implement a *WWAN control function*, used to
>>   manage the use of other WWAN functions, as well as the WWAN unit
>>   itself.
>> - The AP communicates with a WWAN function using a WWAN protocol.
>> - A WWAN physical channel can be *multiplexed*, in which case it
>>   carries the data for one or more *WWAN logical channels*.
> 
> It's unclear to me what "physical" means here. USB Interface or
> Endpoint or PCI Function or SMD channel? Or kernel TTY device?

I'm trying to distinguish between (let's say) "hardware" communication
channels (such as what IPA basically provides) and logical ones,
whose data is multiplexed over a "hardware"/"physical" channel.
Maybe "link" would be a better term for what I referred to as a
physical channel, and then have "channels" be multiplexed over a link.
If you have a good suggestion, please offer it.

But I think yes, USB interface, TTY device, etc. are what I mean.
I wanted to capture the fact that there might be more than one of
these (for example a user space QMI link for control, and IPA link
for data?), and that any one of these might also be multiplexed.

Multiplexing is a pretty basic capability of a network link, and I
think the only reason I called it out separately is to be explicit
that it is needs to be supported.

> For example on Qualcomm-based USB dongles a given USB Interface's
> Endpoint represents a QMAP "IP data" channel which itself could be
> multiplexed into separate "IP data" channels.  Or that USB Endpoint(s)
> could be exposed as a TTY which itself can be MUX-ed dynamically using
> GSM 07.10.

Yeah.  In this case the hardware connection between the AP and the
USB dongle would provide a WWAN link (which I think corresponds to
the QMAP "IP data" channel).  And if you wanted to multiplex that
you would use a multiplexing protocol (like QMAP).  And that protocol
would carry one or more logical channels, each using its own WWAN
protocol.  It sounds like GSM 07.10 would be another WWAN multiplexing
protocol.

> To me "physical" usually means the bus type (PCI, USB, SMD, whatever).
> A Linux hardware driver (IPA, qmi_wwan, option, sierra, etc) binds to
> that physical entity using hardware IDs (USB or PCI VID/PID, devicetree
> properties) and exposes some "WWAN logical communication channels".

(Or perhaps exposes the ability to create "WWAN logical channels.")

"Physical" is probably not a good term.  And perhaps focusing
on the transport isn't right--maybe the focus should mainly be on
the WWAN modem entity.  But one of the things we're trying to
address here is that there might be several distinct "physical"
paths through which the AP and modem can communicate, so a driver's
ability to provide such a path should be a sort of first class
abstraction.

I'm really trying to get this discussion to converge a little, to
have a few anchor points to build on.  I hope we're getting there.

> Those logical channels might be multiplexed and another driver (rmnet)
> could handle exposing the de-muxed logical channels that the muxed
> logical channel carries.
> 
>> - A multiplexed WWAN communication channel uses a *WWAN wultiplexing
>>   protocol*, which is used to separate independent data streams
>>   carrying other WWAN protocols.
>> - A WWAN logical channel carries a bidirectional stream of WWAN
>>   protocol data between an entity on the AP and a WWAN function.
> 
> It *usually* is bidirectional. For example some GPS logical
> communication channels just start spitting out NMEA when you give the
> control function a command. The NMEA ports themselves don't accept any
> input.

That's fine...  I don't think there's anything wrong with a
particular application not using both directions.

>> Does that adequately represent a very high-level picture of what
>> we're trying to manage?
> 
> Yes, pretty well. Thanks for trying to specify it all.

I think we're making progress.  I have some more thoughts but I
think I'll wait until tomorrow to share them.

Thanks a lot Dan.  At some point it might be better to have a
conference call to make better progress, but I'm not suggesting
that yet.

					-Alex

>> And if I understand it right, the purpose of the generic framework
>> being discussed is to define a common mechanism for managing (i.e.,
>> discovering, creating, destroying, querying, configuring, enabling,
>> disabling, etc.) WWAN units and the functions they implement, along
>> with the communication and logical channels used to communicate with
>> them.
> 
> Yes.
> 
> Dan
> 
>> Comments?
>>
>> 					-Alex
> 


^ permalink raw reply

* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Frank Ch. Eigler @ 2019-06-24 20:58 UTC (permalink / raw)
  To: Mathieu Desnoyers
  Cc: Peter Zijlstra, Jessica Yu, linux-kernel, Josh Poimboeuf, jikos,
	mbenes, Petr Mladek, Alexei Starovoitov, Daniel Borkmann,
	Andrew Morton, Robert Richter, rostedt, Ingo Molnar,
	Martin KaFai Lau, Song Liu, Yonghong Song, paulmck,
	Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner,
	oprofile-list, netdev, bpf
In-Reply-To: <320564860.243.1561384864186.JavaMail.zimbra@efficios.com>

Hi -

> > While auditing all module notifiers I noticed a whole bunch of fail
> > wrt the return value. Notifiers have a 'special' return semantics.

From peterz's comments, the patches, it's not obvious to me how one is
to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a
routine success.

> [...]
> I have a similar erroneous module notifier return value pattern
> in lttng-modules as well. I'll go fix it right away. CCing
> Frank Eigler from SystemTAP which AFAIK use a copy of
> lttng-tracepoint.c in their project, which should be fixed
> as well. I'm pasting the lttng-modules fix below.

Sure, following suit.  Thanks.

- FChE

^ permalink raw reply

* Re: [PATCH bpf-next] MAINTAINERS: add reviewer to maintainers entry
From: Jonathan Lemon @ 2019-06-24 20:54 UTC (permalink / raw)
  To: Björn Töpel
  Cc: ast, daniel, netdev, Björn Töpel, magnus.karlsson, bpf,
	linux-kernel
In-Reply-To: <20190624052455.10659-1-bjorn.topel@gmail.com>



On 23 Jun 2019, at 22:24, Björn Töpel wrote:

> From: Björn Töpel <bjorn.topel@intel.com>
>
> Jonathan Lemon has volunteered as an official AF_XDP reviewer. Thank
> you, Jonathan!
>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Acked-by: Jonathan Lemon <jonathan.lemon@gmail.com>

^ permalink raw reply

* Re: [PATCH net-next 02/18] ionic: Add hardware init and device commands
From: Jakub Kicinski @ 2019-06-24 20:53 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-3-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:08 -0700, Shannon Nelson wrote:
> The ionic device has a small set of PCI registers, including a
> device control and data space, and a large set of message
> commands.
> 
> Signed-off-by: Shannon Nelson <snelson@pensando.io>

>  struct ionic {
>  	struct pci_dev *pdev;
>  	struct device *dev;
> +	struct ionic_dev idev;
> +	struct mutex dev_cmd_lock;	/* lock for dev_cmd operations */
> +	struct dentry *dentry;
> +	struct ionic_dev_bar bars[IONIC_BARS_MAX];
> +	unsigned int num_bars;
> +	struct identity ident;
> +	bool is_mgmt_nic;

What's a management NIC?

> +	ionic->is_mgmt_nic =
> +		ent->device == PCI_DEVICE_ID_PENSANDO_IONIC_ETH_MGMT;

You spent time in the docs describing how to use lspci, yet this magic
NIC is not mentioned :)

>  static struct pci_driver ionic_driver = {
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> new file mode 100644
> index 000000000000..e5e45e6bec9d
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_debugfs.c
> @@ -0,0 +1,239 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#include <linux/netdevice.h>
> +
> +#include "ionic.h"
> +#include "ionic_bus.h"
> +#include "ionic_debugfs.h"
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static int blob_open(struct inode *inode, struct file *filp)
> +{
> +	filp->private_data = inode->i_private;
> +	return 0;
> +}
> +
> +static ssize_t blob_read(struct file *filp, char __user *buffer,
> +			 size_t count, loff_t *ppos)
> +{
> +	struct debugfs_blob_wrapper *blob = filp->private_data;
> +
> +	if (*ppos >= blob->size)
> +		return 0;
> +	if (*ppos + count > blob->size)
> +		count = blob->size - *ppos;
> +
> +	if (copy_to_user(buffer, blob->data + *ppos, count))
> +		return -EFAULT;
> +
> +	*ppos += count;
> +
> +	return count;
> +}
> +
> +static ssize_t blob_write(struct file *filp, const char __user *buffer,
> +			  size_t count, loff_t *ppos)
> +{
> +	struct debugfs_blob_wrapper *blob = filp->private_data;
> +
> +	if (*ppos >= blob->size)
> +		return 0;
> +	if (*ppos + count > blob->size)
> +		count = blob->size - *ppos;
> +
> +	if (copy_from_user(blob->data + *ppos, buffer, count))
> +		return -EFAULT;
> +
> +	*ppos += count;
> +
> +	return count;
> +}

Why would you ever have to write to a debugfs blob?  Red flag.

> +static const struct file_operations blob_fops = {
> +	.owner = THIS_MODULE,
> +	.open = blob_open,
> +	.read = blob_read,
> +	.write = blob_write,
> +};
> +
> +struct dentry *debugfs_create_blob(const char *name, umode_t mode,
> +				   struct dentry *parent,
> +				   struct debugfs_blob_wrapper *blob)
> +{
> +	return debugfs_create_file(name, mode | 0200, parent, blob,
> +				   &blob_fops);
> +}
> +
> +static struct dentry *ionic_dir;
> +
> +#define single(name) \
> +static int name##_open(struct inode *inode, struct file *f)	\
> +{								\
> +	return single_open(f, name##_show, inode->i_private);	\
> +}								\
> +								\
> +static const struct file_operations name##_fops = {		\
> +	.owner = THIS_MODULE,					\
> +	.open = name##_open,					\
> +	.read = seq_read,					\
> +	.llseek = seq_lseek,					\
> +	.release = single_release,				\
> +}

DEFINE_SHOW_ATTRIBUTE() and friends.

> +static int bars_show(struct seq_file *seq, void *v)
> +{
> +	struct ionic *ionic = seq->private;
> +	struct ionic_dev_bar *bars = ionic->bars;
> +	unsigned int i;
> +
> +	for (i = 0; i < IONIC_BARS_MAX; i++)
> +		if (bars[i].vaddr)
> +			seq_printf(seq, "BAR%d: len 0x%lx vaddr %pK bus_addr %pad\n",
> +				   i, bars[i].len, bars[i].vaddr,
> +				   &bars[i].bus_addr);

Why? What's the value of this print beyond what's already visible from
PCI subsystem? :S

> +static inline u64 encode_txq_desc_cmd(u8 opcode, u8 flags,
> +				      u8 nsge, u64 addr)
> +{
> +	u64 cmd;
> +
> +	cmd = (opcode & IONIC_TXQ_DESC_OPCODE_MASK) << IONIC_TXQ_DESC_OPCODE_SHIFT;

IIRC you're not a fan of the FIELD_* macros, but let me suggest them
again :)

> +	cmd |= (flags & IONIC_TXQ_DESC_FLAGS_MASK) << IONIC_TXQ_DESC_FLAGS_SHIFT;
> +	cmd |= (nsge & IONIC_TXQ_DESC_NSGE_MASK) << IONIC_TXQ_DESC_NSGE_SHIFT;
> +	cmd |= (addr & IONIC_TXQ_DESC_ADDR_MASK) << IONIC_TXQ_DESC_ADDR_SHIFT;
> +
> +	return cmd;
> +};
> +
> +static inline void decode_txq_desc_cmd(u64 cmd, u8 *opcode, u8 *flags,
> +				       u8 *nsge, u64 *addr)
> +{
> +	*opcode = (cmd >> IONIC_TXQ_DESC_OPCODE_SHIFT) & IONIC_TXQ_DESC_OPCODE_MASK;
> +	*flags = (cmd >> IONIC_TXQ_DESC_FLAGS_SHIFT) & IONIC_TXQ_DESC_FLAGS_MASK;
> +	*nsge = (cmd >> IONIC_TXQ_DESC_NSGE_SHIFT) & IONIC_TXQ_DESC_NSGE_MASK;
> +	*addr = (cmd >> IONIC_TXQ_DESC_ADDR_SHIFT) & IONIC_TXQ_DESC_ADDR_MASK;
> +};
> +
> +#define IONIC_TX_MAX_SG_ELEMS	8
> +#define IONIC_RX_MAX_SG_ELEMS	8

> +/**
> + * struct dev_setattr_cmd - Set Device attributes on the NIC
> + * @opcode:     Opcode
> + * @attr:       Attribute type (enum dev_attr)
> + * @state:      Device state (enum dev_state)
> + * @name:       The bus info, e.g. PCI slot-device-function, 0 terminated

Interesting, why would this be of interest to the device?

> + * @features:   Device features
> + */
> +struct dev_setattr_cmd {
> +	u8     opcode;
> +	u8     attr;
> +	__le16 rsvd;
> +	union {
> +		u8      state;
> +		char    name[IONIC_IFNAMSIZ];
> +		__le64  features;
> +		u8      rsvd2[60];
> +	};
> +};

> +/**
> + * struct lif_getattr_comp - LIF get attr command completion
> + * @status:     The status of the command (enum status_code)
> + * @comp_index: The index in the descriptor ring for which this
> + *              is the completion.
> + * @state:      lif state (enum lif_state)
> + * @name:       The netdev name string, 0 terminated
> + * @mtu:        Mtu
> + * @mac:        Station mac
> + * @features:   Features (enum eth_hw_features)
> + * @color:      Color bit
> + */
> +struct lif_getattr_comp {
> +	u8     status;
> +	u8     rsvd;
> +	__le16 comp_index;
> +	union {
> +		u8      state;
> +		//char    name[IONIC_IFNAMSIZ];

Hi!!

> +		__le32  mtu;
> +		u8      mac[6];
> +		__le64  features;
> +		u8      rsvd2[11];
> +	};
> +	u8     color;
> +};

^ permalink raw reply

* Re: [PATCH bpf-next v5 0/3] xdp: Allow lookup into devmaps before redirect
From: Andrii Nakryiko @ 2019-06-24 20:49 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen
  Cc: Networking, Jesper Dangaard Brouer, Daniel Borkmann,
	Alexei Starovoitov, David Miller, Jonathan Lemon
In-Reply-To: <87y31qepu6.fsf@toke.dk>

On Mon, Jun 24, 2019 at 12:38 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Andrii Nakryiko <andrii.nakryiko@gmail.com> writes:
>
> > On Sat, Jun 22, 2019 at 7:19 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> >>
> >> When using the bpf_redirect_map() helper to redirect packets from XDP, the eBPF
> >> program cannot currently know whether the redirect will succeed, which makes it
> >> impossible to gracefully handle errors. To properly fix this will probably
> >> require deeper changes to the way TX resources are allocated, but one thing that
> >> is fairly straight forward to fix is to allow lookups into devmaps, so programs
> >> can at least know when a redirect is *guaranteed* to fail because there is no
> >> entry in the map. Currently, programs work around this by keeping a shadow map
> >> of another type which indicates whether a map index is valid.
> >>
> >> This series contains two changes that are complementary ways to fix this issue:
> >>
> >> - Moving the map lookup into the bpf_redirect_map() helper (and caching the
> >>   result), so the helper can return an error if no value is found in the map.
> >>   This includes a refactoring of the devmap and cpumap code to not care about
> >>   the index on enqueue.
> >>
> >> - Allowing regular lookups into devmaps from eBPF programs, using the read-only
> >>   flag to make sure they don't change the values.
> >>
> >> The performance impact of the series is negligible, in the sense that I cannot
> >> measure it because the variance between test runs is higher than the difference
> >> pre/post series.
> >>
> >> Changelog:
> >>
> >> v5:
> >>   - Rebase on latest bpf-next.
> >>   - Update documentation for bpf_redirect_map() with the new meaning of flags.
> >>
> >> v4:
> >>   - Fix a few nits from Andrii
> >>   - Lose the #defines in bpf.h and just compare the flags argument directly to
> >>     XDP_TX in bpf_xdp_redirect_map().
> >>
> >> v3:
> >>   - Adopt Jonathan's idea of using the lower two bits of the flag value as the
> >>     return code.
> >>   - Always do the lookup, and cache the result for use in xdp_do_redirect(); to
> >>     achieve this, refactor the devmap and cpumap code to get rid the bitmap for
> >>     selecting which devices to flush.
> >>
> >> v2:
> >>   - For patch 1, make it clear that the change works for any map type.
> >>   - For patch 2, just use the new BPF_F_RDONLY_PROG flag to make the return
> >>     value read-only.
> >>
> >> ---
> >>
> >> Toke Høiland-Jørgensen (3):
> >>       devmap/cpumap: Use flush list instead of bitmap
> >>       bpf_xdp_redirect_map: Perform map lookup in eBPF helper
> >>       devmap: Allow map lookups from eBPF
> >>
> >>
> >>  include/linux/filter.h   |    1
> >>  include/uapi/linux/bpf.h |    7 ++-
> >>  kernel/bpf/cpumap.c      |  106 ++++++++++++++++++++-----------------------
> >>  kernel/bpf/devmap.c      |  113 ++++++++++++++++++++++------------------------
> >>  kernel/bpf/verifier.c    |    7 +--
> >>  net/core/filter.c        |   29 +++++-------
> >>  6 files changed, 123 insertions(+), 140 deletions(-)
> >>
> >
> >
> > Looks like you forgot to add my Acked-by's for your patches?
>
> Ah yes, did not carry those forward for the individual patches, my
> apologies. Could you perhaps be persuaded to send a new one (I believe a
> response to the cover letter acking the whole series would suffice)?
> I'll make sure to add the carrying forward of acks into my workflow in
> the future :)

I don't think patchworks captures ack's from cover letter, but let's
give it a go:

Acked-by: Andrii Nakryiko <andriin@fb.com>

>
> -Toke

^ permalink raw reply

* [PATCH v2 net-next] ipv6: Convert gateway validation to use fib6_info
From: David Ahern @ 2019-06-24 20:44 UTC (permalink / raw)
  To: davem; +Cc: netdev, kafai, weiwan, David Ahern

From: David Ahern <dsahern@gmail.com>

Gateway validation does not need a dst_entry, it only needs the fib
entry to validate the gateway resolution and egress device. So,
convert ip6_nh_lookup_table from ip6_pol_route to fib6_table_lookup
and ip6_route_check_nh to use fib6_lookup over rt6_lookup.

ip6_pol_route is a call to fib6_table_lookup and if successful a call
to fib6_select_path. From there the exception cache is searched for an
entry or a dst_entry is created to return to the caller. The exception
entry is not relevant for gateway validation, so what matters are the
calls to fib6_table_lookup and then fib6_select_path.

Similarly, rt6_lookup can be replaced with a call to fib6_lookup with
RT6_LOOKUP_F_IFACE set in flags. Again, the exception cache search is
not relevant, only the lookup with path selection. The primary difference
in the lookup paths is the use of rt6_select with fib6_lookup versus
rt6_device_match with rt6_lookup. When you remove complexities in the
rt6_select path, e.g.,
1. saddr is not set for gateway validation, so RT6_LOOKUP_F_HAS_SADDR
   is not relevant
2. rt6_check_neigh is not called so that removes the RT6_NUD_FAIL_DO_RR
   return and round-robin logic.

the code paths are believed to be equivalent for the given use case -
validate the gateway and optionally given the device. Furthermore, it
aligns the validation with onlink code path and the lookup path actually
used for rx and tx.

Adjust the users, ip6_route_check_nh_onlink and ip6_route_check_nh to
handle a fib6_info vs a rt6_info when performing validation checks.

Existing selftests fib-onlink-tests.sh and fib_tests.sh are used to
verify the changes.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
v2
- use in6_dev_get versus __in6_dev_get + in6_dev_hold (comment from Wei)
- updated commit message

 net/ipv6/route.c | 118 ++++++++++++++++++++++++++-----------------------------
 1 file changed, 56 insertions(+), 62 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index be5e65c97652..5fe0fd6f2909 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3144,10 +3144,9 @@ static int ip6_dst_gc(struct dst_ops *ops)
 	return entries > rt_max_size;
 }
 
-static struct rt6_info *ip6_nh_lookup_table(struct net *net,
-					    struct fib6_config *cfg,
-					    const struct in6_addr *gw_addr,
-					    u32 tbid, int flags)
+static int ip6_nh_lookup_table(struct net *net, struct fib6_config *cfg,
+			       const struct in6_addr *gw_addr, u32 tbid,
+			       int flags, struct fib6_result *res)
 {
 	struct flowi6 fl6 = {
 		.flowi6_oif = cfg->fc_ifindex,
@@ -3155,25 +3154,23 @@ static struct rt6_info *ip6_nh_lookup_table(struct net *net,
 		.saddr = cfg->fc_prefsrc,
 	};
 	struct fib6_table *table;
-	struct rt6_info *rt;
+	int err;
 
 	table = fib6_get_table(net, tbid);
 	if (!table)
-		return NULL;
+		return -EINVAL;
 
 	if (!ipv6_addr_any(&cfg->fc_prefsrc))
 		flags |= RT6_LOOKUP_F_HAS_SADDR;
 
 	flags |= RT6_LOOKUP_F_IGNORE_LINKSTATE;
-	rt = ip6_pol_route(net, table, cfg->fc_ifindex, &fl6, NULL, flags);
 
-	/* if table lookup failed, fall back to full lookup */
-	if (rt == net->ipv6.ip6_null_entry) {
-		ip6_rt_put(rt);
-		rt = NULL;
-	}
+	err = fib6_table_lookup(net, table, cfg->fc_ifindex, &fl6, res, flags);
+	if (!err && res->f6i != net->ipv6.fib6_null_entry)
+		fib6_select_path(net, res, &fl6, cfg->fc_ifindex,
+				 cfg->fc_ifindex != 0, NULL, flags);
 
-	return rt;
+	return err;
 }
 
 static int ip6_route_check_nh_onlink(struct net *net,
@@ -3181,29 +3178,19 @@ static int ip6_route_check_nh_onlink(struct net *net,
 				     const struct net_device *dev,
 				     struct netlink_ext_ack *extack)
 {
-	u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
+	u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
 	const struct in6_addr *gw_addr = &cfg->fc_gateway;
-	u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
-	struct fib6_info *from;
-	struct rt6_info *grt;
+	struct fib6_result res = {};
 	int err;
 
-	err = 0;
-	grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
-	if (grt) {
-		rcu_read_lock();
-		from = rcu_dereference(grt->from);
-		if (!grt->dst.error &&
-		    /* ignore match if it is the default route */
-		    from && !ipv6_addr_any(&from->fib6_dst.addr) &&
-		    (grt->rt6i_flags & flags || dev != grt->dst.dev)) {
-			NL_SET_ERR_MSG(extack,
-				       "Nexthop has invalid gateway or device mismatch");
-			err = -EINVAL;
-		}
-		rcu_read_unlock();
-
-		ip6_rt_put(grt);
+	err = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0, &res);
+	if (!err && !(res.fib6_flags & RTF_REJECT) &&
+	    /* ignore match if it is the default route */
+	    !ipv6_addr_any(&res.f6i->fib6_dst.addr) &&
+	    (res.fib6_type != RTN_UNICAST || dev != res.nh->fib_nh_dev)) {
+		NL_SET_ERR_MSG(extack,
+			       "Nexthop has invalid gateway or device mismatch");
+		err = -EINVAL;
 	}
 
 	return err;
@@ -3216,47 +3203,50 @@ static int ip6_route_check_nh(struct net *net,
 {
 	const struct in6_addr *gw_addr = &cfg->fc_gateway;
 	struct net_device *dev = _dev ? *_dev : NULL;
-	struct rt6_info *grt = NULL;
+	int flags = RT6_LOOKUP_F_IFACE;
+	struct fib6_result res = {};
 	int err = -EHOSTUNREACH;
 
 	if (cfg->fc_table) {
-		int flags = RT6_LOOKUP_F_IFACE;
-
-		grt = ip6_nh_lookup_table(net, cfg, gw_addr,
-					  cfg->fc_table, flags);
-		if (grt) {
-			if (grt->rt6i_flags & RTF_GATEWAY ||
-			    (dev && dev != grt->dst.dev)) {
-				ip6_rt_put(grt);
-				grt = NULL;
-			}
-		}
+		err = ip6_nh_lookup_table(net, cfg, gw_addr,
+					  cfg->fc_table, flags, &res);
+		/* gw_addr can not require a gateway or resolve to a reject
+		 * route. If a device is given, it must match the result.
+		 */
+		if (err || res.fib6_flags & RTF_REJECT ||
+		    res.nh->fib_nh_gw_family ||
+		    (dev && dev != res.nh->fib_nh_dev))
+			err = -EHOSTUNREACH;
 	}
 
-	if (!grt)
-		grt = rt6_lookup(net, gw_addr, NULL, cfg->fc_ifindex, NULL, 1);
+	if (err < 0) {
+		struct flowi6 fl6 = {
+			.flowi6_oif = cfg->fc_ifindex,
+			.daddr = *gw_addr,
+		};
 
-	if (!grt)
-		goto out;
+		err = fib6_lookup(net, cfg->fc_ifindex, &fl6, &res, flags);
+		if (err || res.fib6_flags & RTF_REJECT ||
+		    res.nh->fib_nh_gw_family)
+			err = -EHOSTUNREACH;
+
+		if (err)
+			return err;
+
+		fib6_select_path(net, &res, &fl6, cfg->fc_ifindex,
+				 cfg->fc_ifindex != 0, NULL, flags);
+	}
 
+	err = 0;
 	if (dev) {
-		if (dev != grt->dst.dev) {
-			ip6_rt_put(grt);
-			goto out;
-		}
+		if (dev != res.nh->fib_nh_dev)
+			err = -EHOSTUNREACH;
 	} else {
-		*_dev = dev = grt->dst.dev;
-		*idev = grt->rt6i_idev;
+		*_dev = dev = res.nh->fib_nh_dev;
 		dev_hold(dev);
-		in6_dev_hold(grt->rt6i_idev);
+		*idev = in6_dev_get(dev);
 	}
 
-	if (!(grt->rt6i_flags & RTF_GATEWAY))
-		err = 0;
-
-	ip6_rt_put(grt);
-
-out:
 	return err;
 }
 
@@ -3297,11 +3287,15 @@ static int ip6_validate_gw(struct net *net, struct fib6_config *cfg,
 			goto out;
 		}
 
+		rcu_read_lock();
+
 		if (cfg->fc_flags & RTNH_F_ONLINK)
 			err = ip6_route_check_nh_onlink(net, cfg, dev, extack);
 		else
 			err = ip6_route_check_nh(net, cfg, _dev, idev);
 
+		rcu_read_unlock();
+
 		if (err)
 			goto out;
 	}
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH] bpf: hide do_bpf_send_signal when unused
From: Yonghong Song @ 2019-06-24 20:37 UTC (permalink / raw)
  To: Steven Rostedt, Alexei Starovoitov, arnd@arndb.de
  Cc: Matt Mullins, Song Liu, linux-kernel@vger.kernel.org,
	daniel@iogearbox.net, bpf@vger.kernel.org, ast@kernel.org,
	mingo@redhat.com, netdev@vger.kernel.org, Martin Lau,
	Andrii Nakryiko
In-Reply-To: <20190617201850.010a4cf6@gandalf.local.home>



On 6/17/19 5:18 PM, Steven Rostedt wrote:
> On Mon, 17 Jun 2019 16:27:33 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> 
>> On Mon, Jun 17, 2019 at 4:13 PM Matt Mullins <mmullins@fb.com> wrote:
>>>>
>>>> The bug (really just a warning) reported is exactly here.
>>>
>>> I don't think bpf_send_signal is tied to modules at all;
>>> send_signal_irq_work_init and the corresponding initcall should be
>>> moved outside that #ifdef.
>>
>> right. I guess send_signal_irq_work_init was accidentally placed
>> after bpf_event_init and happened to be within that ifdef.
>> Should definitely be outside.
> 
> So Arnd did find a bug. Just the wrong solution ;-)
> 
> -- Steve

Hi, Arnd,

The following change can fix the issue.

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index c102c240bb0b..ca1255d14576 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1431,6 +1431,20 @@ int bpf_get_perf_event_info(const struct 
perf_event *event, u32 *prog_id,
         return err;
  }

+static int __init send_signal_irq_work_init(void)
+{
+       int cpu;
+       struct send_signal_irq_work *work;
+
+       for_each_possible_cpu(cpu) {
+               work = per_cpu_ptr(&send_signal_work, cpu);
+               init_irq_work(&work->irq_work, do_bpf_send_signal);
+       }
+       return 0;
+}
+
+subsys_initcall(send_signal_irq_work_init);
+
  #ifdef CONFIG_MODULES
  static int bpf_event_notify(struct notifier_block *nb, unsigned long op,
                             void *module)
@@ -1478,18 +1492,5 @@ static int __init bpf_event_init(void)
         return 0;
  }

-static int __init send_signal_irq_work_init(void)
-{
-       int cpu;
-       struct send_signal_irq_work *work;
-
-       for_each_possible_cpu(cpu) {
-               work = per_cpu_ptr(&send_signal_work, cpu);
-               init_irq_work(&work->irq_work, do_bpf_send_signal);
-       }
-       return 0;
-}
-
  fs_initcall(bpf_event_init);
-subsys_initcall(send_signal_irq_work_init);
  #endif /* CONFIG_MODULES */

Could you submit a new revision? Thanks!

Yonghong

^ permalink raw reply related

* Re: [PATCH v7 0/6] Add support for Orange Pi 3
From: Ondřej Jirman @ 2019-06-24 20:28 UTC (permalink / raw)
  To: David Miller
  Cc: linux-sunxi, maxime.ripard, wens, robh+dt, jernej.skrabec,
	airlied, daniel, mark.rutland, peppe.cavallaro, alexandre.torgue,
	joabreu, mcoquelin.stm32, dri-devel, devicetree, linux-arm-kernel,
	linux-kernel, netdev, linux-stm32
In-Reply-To: <20190624.132456.2013417744691373807.davem@davemloft.net>

On Mon, Jun 24, 2019 at 01:24:56PM -0700, David Miller wrote:
> From: Ondřej Jirman <megous@megous.com>
> Date: Mon, 24 Jun 2019 19:46:37 +0200
> 
> > This series was even longer before, with patches all around for various
> > maintainers. I'd expect that relevant maintainers pick the range of patches
> > meant for them. I don't know who's exactly responsible for what, but I think,
> > this should work:
> > 
> > - 2 stmmac patches should go together via some networking tree (is there
> >   something specific for stmmac?)
> > - all DTS patches should go via sunxi
> > - hdmi patches via some drm tree
> 
> Thank you.  So I'll merge the first two patches that touch the stmmac
> driver via my net-next tree.

Thank you.

regards,
	Ondrej

^ permalink raw reply

* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: David Miller @ 2019-06-24 20:27 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: ndesaulniers, netdev, lkp
In-Reply-To: <3d7c16c4-9c3e-d18c-aad4-6583216ea457@6wind.com>

From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Mon, 24 Jun 2019 20:18:37 +0200

> Le 24/06/2019 à 19:37, Nick Desaulniers a écrit :
> [snip]
>> 
>> The author stated that this patch was no functional change.  Nicolas,
>> it can be helpful to include compiler warnings in the commit message
>> when sending warning fixes, but it's not a big deal.  Thanks for
>> sending the patches.
>> 
> Yep, but I was not aware of this compilation warning. As explained in the commit
> log, the goal of this patch was to prepare the next one.

Yeah, don't worry about it.

^ permalink raw reply

* Re: [PATCH v7 0/6] Add support for Orange Pi 3
From: David Miller @ 2019-06-24 20:24 UTC (permalink / raw)
  To: megous
  Cc: linux-sunxi, maxime.ripard, wens, robh+dt, jernej.skrabec,
	airlied, daniel, mark.rutland, peppe.cavallaro, alexandre.torgue,
	joabreu, mcoquelin.stm32, dri-devel, devicetree, linux-arm-kernel,
	linux-kernel, netdev, linux-stm32
In-Reply-To: <20190624174637.6sznc5ifiuh4c3sm@core.my.home>

From: Ondřej Jirman <megous@megous.com>
Date: Mon, 24 Jun 2019 19:46:37 +0200

> This series was even longer before, with patches all around for various
> maintainers. I'd expect that relevant maintainers pick the range of patches
> meant for them. I don't know who's exactly responsible for what, but I think,
> this should work:
> 
> - 2 stmmac patches should go together via some networking tree (is there
>   something specific for stmmac?)
> - all DTS patches should go via sunxi
> - hdmi patches via some drm tree

Thank you.  So I'll merge the first two patches that touch the stmmac
driver via my net-next tree.

^ permalink raw reply

* Re: [PATCH net-next 00/18] Add ionic driver
From: Jakub Kicinski @ 2019-06-24 20:19 UTC (permalink / raw)
  To: David Miller; +Cc: Shannon Nelson, netdev, Andrew Lunn
In-Reply-To: <20190620202424.23215-1-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:06 -0700, Shannon Nelson wrote:
>  28 files changed, 9970 insertions(+)

Dave, could we consider setting a LoC limit for series and patches?
I know this is a new driver, but there's gotta be a way to split 
this up more, even if it's painful for the submitter :S

All the debugfs stuff shouldn't be necessary in the first version,
just looking at first 2 patches...

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-06-24 20:15 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Daniel Borkmann, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <CALCETrWmZX3R1L88Gz9vLY68gcK8zSXL4cA4GqAzQoyqSR7rRQ@mail.gmail.com>

On Mon, Jun 24, 2019 at 1:09 PM Andy Lutomirski <luto@kernel.org> wrote:

> I'm confused.  I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?

Hmm. I think the thinking here was around exfiltration mechanisms, but
if the read is blocked then that seems less likely. This seems to
trace back to http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003545.html
- Joey, do you know the reasoning here?

^ permalink raw reply

* Re: [PATCH net-next 02/18] ionic: Add hardware init and device commands
From: Jakub Kicinski @ 2019-06-24 20:13 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: Andrew Lunn, netdev
In-Reply-To: <65461426-92d8-cd87-942d-1fd82bd64fe4@pensando.io>

On Fri, 21 Jun 2019 15:22:22 -0700, Shannon Nelson wrote:
> >> +static int identity_show(struct seq_file *seq, void *v)
> >> +{
> >> +	struct ionic *ionic = seq->private;
> >> +	struct identity *ident = &ionic->ident;
> >> +	struct ionic_dev *idev = &ionic->idev;
> >> +
> >> +	seq_printf(seq, "asic_type:        0x%x\n", idev->dev_info.asic_type);
> >> +	seq_printf(seq, "asic_rev:         0x%x\n", idev->dev_info.asic_rev);
> >> +	seq_printf(seq, "serial_num:       %s\n", idev->dev_info.serial_num);
> >> +	seq_printf(seq, "fw_version:       %s\n", idev->dev_info.fw_version);
> >> +	seq_printf(seq, "fw_status:        0x%x\n",
> >> +		   ioread8(&idev->dev_info_regs->fw_status));
> >> +	seq_printf(seq, "fw_heartbeat:     0x%x\n",
> >> +		   ioread32(&idev->dev_info_regs->fw_heartbeat));  
> > devlink just gained a much more flexible version of ethtool -i. Please
> > remove all this and use that.  
> Yes, we intend to add a devlink interface, it just isn't in this first 
> patchset, which is already plenty big.

Please take this out of your patch set, we can't be expected to merge
debugfs implementation of what has proper APIs :/

^ permalink raw reply

* Re: [PATCH V34 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Andy Lutomirski @ 2019-06-24 20:08 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Daniel Borkmann, James Morris, LSM List,
	Linux Kernel Mailing List, Linux API, David Howells,
	Alexei Starovoitov, Network Development, Chun-Yi Lee, Jann Horn,
	bpf
In-Reply-To: <CACdnJuvR2bn3y3fYzg06GWXXgAGjgED2Dfa5g0oAwJ28qCCqBg@mail.gmail.com>

On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <mjg59@google.com> wrote:
>
> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > > From: David Howells <dhowells@redhat.com>
> > >
> > > There are some bpf functions can be used to read kernel memory:
> >
> > Nit: that
>
> Fixed.
>
> > > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk.  These allow
> >
> > Please explain how bpf_probe_write_user reads kernel memory ... ?!
>
> Ha.
>
> > > private keys in kernel memory (e.g. the hibernation image signing key) to
> > > be read by an eBPF program and kernel memory to be altered without
> >
> > ... and while we're at it, also how they allow "kernel memory to be
> > altered without restriction". I've been pointing this false statement
> > out long ago.
>
> Yup. How's the following description:
>
>     bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>
>     There are some bpf functions that can be used to read kernel memory and
>     exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>     bpf_trace_printk.  These could be abused to (eg) allow private
> keys in kernel
>     memory to be leaked. Disable them if the kernel has been locked
> down in confidentiality
>     mode.

I'm confused.  I understand why we're restricting bpf_probe_read().
Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
though?

--Andy

^ permalink raw reply

* Re: [PATCH bpf-next] MAINTAINERS: add reviewer to maintainers entry
From: Song Liu @ 2019-06-24 20:07 UTC (permalink / raw)
  To: Björn Töpel
  Cc: Alexei Starovoitov, Daniel Borkmann, Networking,
	Björn Töpel, Magnus Karlsson, bpf, open list,
	jonathan.lemon
In-Reply-To: <20190624052455.10659-1-bjorn.topel@gmail.com>

On Sun, Jun 23, 2019 at 10:45 PM Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> From: Björn Töpel <bjorn.topel@intel.com>
>
> Jonathan Lemon has volunteered as an official AF_XDP reviewer. Thank
> you, Jonathan!

Thanks Jonathan! Please reply with your Acked-by.

Thanks,
Song

>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 0cfe98a6761a..dd875578d53c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17284,6 +17284,7 @@ N:      xdp
>  XDP SOCKETS (AF_XDP)
>  M:     Björn Töpel <bjorn.topel@intel.com>
>  M:     Magnus Karlsson <magnus.karlsson@intel.com>
> +R:     Jonathan Lemon <jonathan.lemon@gmail.com>
>  L:     netdev@vger.kernel.org
>  L:     bpf@vger.kernel.org
>  S:     Maintained
> --
> 2.20.1
>

^ permalink raw reply

* Re: [PATCH net-next 01/18] ionic: Add basic framework for IONIC Network device driver
From: Jakub Kicinski @ 2019-06-24 20:07 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: Andrew Lunn, netdev
In-Reply-To: <7f1fcda2-dce4-feb6-ec3a-c54bfb691e5d@pensando.io>

On Fri, 21 Jun 2019 15:13:31 -0700, Shannon Nelson wrote:
> >> +#define DRV_VERSION		"0.11.0-k"  
> > DRV_VERSION is pretty useless. What you really want to know is the
> > kernel git tree and commit. The big distributions might backport this
> > version of the driver back to the old kernel with a million
> > patches. At which point 0.11.0-k tells you nothing much.  
> Yes, any version numbering thing from the big distros is put into 
> question, but I find this number useful to me for tracking what has been 
> put into the upstream kernel.  This plus the full kernel version gives 
> me a pretty good idea of what I'm looking at.

Still, we strongly encourage ditching the driver version.  
It encourages upstream first development model among other benefits.

^ permalink raw reply

* Re: [PATCH net-next 01/18] ionic: Add basic framework for IONIC Network device driver
From: Jakub Kicinski @ 2019-06-24 20:03 UTC (permalink / raw)
  To: Shannon Nelson; +Cc: netdev
In-Reply-To: <20190620202424.23215-2-snelson@pensando.io>

On Thu, 20 Jun 2019 13:24:07 -0700, Shannon Nelson wrote:
> diff --git a/Documentation/networking/device_drivers/pensando/ionic.rst b/Documentation/networking/device_drivers/pensando/ionic.rst
> new file mode 100644
> index 000000000000..84bdf682052b
> --- /dev/null
> +++ b/Documentation/networking/device_drivers/pensando/ionic.rst
> @@ -0,0 +1,75 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +==========================================================
> +Linux* Driver for the Pensando(R) Ethernet adapter family
> +==========================================================
> +
> +Pensando Linux Ethernet driver.
> +Copyright(c) 2019 Pensando Systems, Inc
> +
> +Contents
> +========
> +
> +- Identifying the Adapter
> +- Special Features
> +- Support
> +
> +

nit: all instances of multiple empty lines in the docs look a bit
unnecessary

^ permalink raw reply

* [PATCH ipsec-next] xfrm: remove get_mtu indirection from xfrm_type
From: Florian Westphal @ 2019-06-24 20:04 UTC (permalink / raw)
  To: netdev; +Cc: Florian Westphal

esp4_get_mtu and esp6_get_mtu are exactly the same, the only difference
is a single sizeof() (ipv4 vs. ipv6 header).

Merge both into xfrm_state_mtu() and remove the indirection.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 include/net/xfrm.h     |  4 +---
 net/ipv4/esp4.c        | 27 +--------------------------
 net/ipv6/esp6.c        | 20 +-------------------
 net/xfrm/xfrm_device.c |  5 ++---
 net/xfrm/xfrm_state.c  | 34 +++++++++++++++++++++++++++++-----
 5 files changed, 34 insertions(+), 56 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 56b31676e330..b22db30c3d88 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -404,8 +404,6 @@ struct xfrm_type {
 	int			(*reject)(struct xfrm_state *, struct sk_buff *,
 					  const struct flowi *);
 	int			(*hdr_offset)(struct xfrm_state *, struct sk_buff *, u8 **);
-	/* Estimate maximal size of result of transformation of a dgram */
-	u32			(*get_mtu)(struct xfrm_state *, int size);
 };
 
 int xfrm_register_type(const struct xfrm_type *type, unsigned short family);
@@ -1546,7 +1544,7 @@ void xfrm_sad_getinfo(struct net *net, struct xfrmk_sadinfo *si);
 void xfrm_spd_getinfo(struct net *net, struct xfrmk_spdinfo *si);
 u32 xfrm_replay_seqhi(struct xfrm_state *x, __be32 net_seq);
 int xfrm_init_replay(struct xfrm_state *x);
-int xfrm_state_mtu(struct xfrm_state *x, int mtu);
+u32 xfrm_state_mtu(struct xfrm_state *x, int mtu);
 int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload);
 int xfrm_init_state(struct xfrm_state *x);
 int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type);
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index c06562aded11..5c967764041f 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -33,8 +33,6 @@ struct esp_output_extra {
 
 #define ESP_SKB_CB(__skb) ((struct esp_skb_cb *)&((__skb)->cb[0]))
 
-static u32 esp4_get_mtu(struct xfrm_state *x, int mtu);
-
 /*
  * Allocate an AEAD request structure with extra space for SG and IV.
  *
@@ -506,7 +504,7 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
 		struct xfrm_dst *dst = (struct xfrm_dst *)skb_dst(skb);
 		u32 padto;
 
-		padto = min(x->tfcpad, esp4_get_mtu(x, dst->child_mtu_cached));
+		padto = min(x->tfcpad, xfrm_state_mtu(x, dst->child_mtu_cached));
 		if (skb->len < padto)
 			esp.tfclen = padto - skb->len;
 	}
@@ -788,28 +786,6 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
 	return err;
 }
 
-static u32 esp4_get_mtu(struct xfrm_state *x, int mtu)
-{
-	struct crypto_aead *aead = x->data;
-	u32 blksize = ALIGN(crypto_aead_blocksize(aead), 4);
-	unsigned int net_adj;
-
-	switch (x->props.mode) {
-	case XFRM_MODE_TRANSPORT:
-	case XFRM_MODE_BEET:
-		net_adj = sizeof(struct iphdr);
-		break;
-	case XFRM_MODE_TUNNEL:
-		net_adj = 0;
-		break;
-	default:
-		BUG();
-	}
-
-	return ((mtu - x->props.header_len - crypto_aead_authsize(aead) -
-		 net_adj) & ~(blksize - 1)) + net_adj - 2;
-}
-
 static int esp4_err(struct sk_buff *skb, u32 info)
 {
 	struct net *net = dev_net(skb->dev);
@@ -1035,7 +1011,6 @@ static const struct xfrm_type esp_type =
 	.flags		= XFRM_TYPE_REPLAY_PROT,
 	.init_state	= esp_init_state,
 	.destructor	= esp_destroy,
-	.get_mtu	= esp4_get_mtu,
 	.input		= esp_input,
 	.output		= esp_output,
 };
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b6c6b3e08836..a3b403ba8f8f 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -41,8 +41,6 @@ struct esp_skb_cb {
 
 #define ESP_SKB_CB(__skb) ((struct esp_skb_cb *)&((__skb)->cb[0]))
 
-static u32 esp6_get_mtu(struct xfrm_state *x, int mtu);
-
 /*
  * Allocate an AEAD request structure with extra space for SG and IV.
  *
@@ -447,7 +445,7 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
 		struct xfrm_dst *dst = (struct xfrm_dst *)skb_dst(skb);
 		u32 padto;
 
-		padto = min(x->tfcpad, esp6_get_mtu(x, dst->child_mtu_cached));
+		padto = min(x->tfcpad, xfrm_state_mtu(x, dst->child_mtu_cached));
 		if (skb->len < padto)
 			esp.tfclen = padto - skb->len;
 	}
@@ -687,21 +685,6 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
 	return ret;
 }
 
-static u32 esp6_get_mtu(struct xfrm_state *x, int mtu)
-{
-	struct crypto_aead *aead = x->data;
-	u32 blksize = ALIGN(crypto_aead_blocksize(aead), 4);
-	unsigned int net_adj;
-
-	if (x->props.mode != XFRM_MODE_TUNNEL)
-		net_adj = sizeof(struct ipv6hdr);
-	else
-		net_adj = 0;
-
-	return ((mtu - x->props.header_len - crypto_aead_authsize(aead) -
-		 net_adj) & ~(blksize - 1)) + net_adj - 2;
-}
-
 static int esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		    u8 type, u8 code, int offset, __be32 info)
 {
@@ -919,7 +902,6 @@ static const struct xfrm_type esp6_type = {
 	.flags		= XFRM_TYPE_REPLAY_PROT,
 	.init_state	= esp6_init_state,
 	.destructor	= esp6_destroy,
-	.get_mtu	= esp6_get_mtu,
 	.input		= esp6_input,
 	.output		= esp6_output,
 	.hdr_offset	= xfrm6_find_1stfragopt,
diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index b24cd86a02c3..f10a70388f72 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -275,9 +275,8 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 		return false;
 
 	if ((!dev || (dev == xfrm_dst_path(dst)->dev)) &&
-	    (!xdst->child->xfrm && x->type->get_mtu)) {
-		mtu = x->type->get_mtu(x, xdst->child_mtu_cached);
-
+	    (!xdst->child->xfrm)) {
+		mtu = xfrm_state_mtu(x, xdst->child_mtu_cached);
 		if (skb->len <= mtu)
 			goto ok;
 
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index fd51737f9f17..c6f3c4a1bd99 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -27,6 +27,8 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 
+#include <crypto/aead.h>
+
 #include "xfrm_hash.h"
 
 #define xfrm_state_deref_prot(table, net) \
@@ -2403,16 +2405,38 @@ void xfrm_state_delete_tunnel(struct xfrm_state *x)
 }
 EXPORT_SYMBOL(xfrm_state_delete_tunnel);
 
-int xfrm_state_mtu(struct xfrm_state *x, int mtu)
+u32 xfrm_state_mtu(struct xfrm_state *x, int mtu)
 {
 	const struct xfrm_type *type = READ_ONCE(x->type);
+	struct crypto_aead *aead;
+	u32 blksize, net_adj = 0;
+
+	if (x->km.state != XFRM_STATE_VALID ||
+	    !type || type->proto != IPPROTO_ESP)
+		return mtu - x->props.header_len;
+
+	aead = x->data;
+	blksize = ALIGN(crypto_aead_blocksize(aead), 4);
 
-	if (x->km.state == XFRM_STATE_VALID &&
-	    type && type->get_mtu)
-		return type->get_mtu(x, mtu);
+	switch (x->props.mode) {
+	case XFRM_MODE_TRANSPORT:
+	case XFRM_MODE_BEET:
+		if (x->props.family == AF_INET)
+			net_adj = sizeof(struct iphdr);
+		else if (x->props.family == AF_INET6)
+			net_adj = sizeof(struct ipv6hdr);
+		break;
+	case XFRM_MODE_TUNNEL:
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		break;
+	}
 
-	return mtu - x->props.header_len;
+	return ((mtu - x->props.header_len - crypto_aead_authsize(aead) -
+		 net_adj) & ~(blksize - 1)) + net_adj - 2;
 }
+EXPORT_SYMBOL_GPL(xfrm_state_mtu);
 
 int __xfrm_init_state(struct xfrm_state *x, bool init_replay, bool offload)
 {
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH net-next] ipv4: enable route flushing in network namespaces
From: Christian Brauner @ 2019-06-24 19:59 UTC (permalink / raw)
  To: David Ahern, davem, kuznet, yoshfuji, netdev; +Cc: linux-kernel
In-Reply-To: <56ed92eb-14db-789a-c226-cdf8a5862e61@gmail.com>

On June 24, 2019 9:49:33 PM GMT+02:00, David Ahern <dsahern@gmail.com> wrote:
>On 6/24/19 7:29 AM, Christian Brauner wrote:
>> Tools such as vpnc try to flush routes when run inside network
>> namespaces by writing 1 into /proc/sys/net/ipv4/route/flush. This
>> currently does not work because flush is not enabled in non-initial
>> network namespaces.
>> Since routes are per network namespace it is safe to enable
>> /proc/sys/net/ipv4/route/flush in there.
>> 
>> Link: https://github.com/lxc/lxd/issues/4257
>> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
>> ---
>>  net/ipv4/route.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>> 
>
>why not teach vpnc to use rtnetlink and then add a flush option to
>RTM_DELROUTE?

I think that if you can do it unprivileged through netlink
you should also allow it through sysctls.
Even the original commit references it
to make it possible to enable the sysctls
1-by-1 as needed.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox