Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH v2 bpf-next 0/2] verifier liveness simplification
From: Edward Cree @ 2018-09-28 13:36 UTC (permalink / raw)
  To: Jiong Wang, ast, daniel; +Cc: netdev
In-Reply-To: <0252cca7-82e4-24d3-8682-e1a613d6cd78@netronome.com>

On 26/09/18 23:16, Jiong Wang wrote:
> On 22/08/2018 20:00, Edward Cree wrote:
>> In the future this idea may be extended to form use-def chains.
>
>   1. instruction level use->def chain
>
>      - new use->def chains for each instruction. one eBPF insn could have two
>        uses at maximum.
I was thinking of something a lot weaker/simpler, just making
    ld rX, rY
 copy rY.parent into rX.parent and not read-mark rY (whereas actual
 arithmetic, pointer deref etc. would still create read marks).
But what you've described sounds interesting; perhaps it would also
 help later with loop-variable handling?

-Ed

^ permalink raw reply

* Re: [PATCHv3 bpf-next 04/12] bpf: Add PTR_TO_SOCKET verifier type
From: Daniel Borkmann @ 2018-09-28 13:34 UTC (permalink / raw)
  To: Joe Stringer
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez
In-Reply-To: <20180927232659.14348-5-joe@wand.net.nz>

On 09/28/2018 01:26 AM, Joe Stringer wrote:
> Teach the verifier a little bit about a new type of pointer, a
> PTR_TO_SOCKET. This pointer type is accessed from BPF through the
> 'struct bpf_sock' structure.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
[...]
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 72db8afb7cb6..057af3dc9f08 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5394,23 +5394,29 @@ static bool __sock_filter_check_size(int off, int size,
>  	return size == size_default;
>  }
>  
> -static bool sock_filter_is_valid_access(int off, int size,
> -					enum bpf_access_type type,
> -					const struct bpf_prog *prog,
> -					struct bpf_insn_access_aux *info)
> +bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
> +			      struct bpf_insn_access_aux *info)
>  {
>  	if (off < 0 || off >= sizeof(struct bpf_sock))
>  		return false;
>  	if (off % size != 0)
>  		return false;
> -	if (!__sock_filter_check_attach_type(off, type,
> -					     prog->expected_attach_type))
> -		return false;
>  	if (!__sock_filter_check_size(off, size, info))
>  		return false;
>  	return true;
>  }
>  
> +static bool sock_filter_is_valid_access(int off, int size,
> +					enum bpf_access_type type,
> +					const struct bpf_prog *prog,
> +					struct bpf_insn_access_aux *info)
> +{
> +	if (!__sock_filter_check_attach_type(off, type,
> +					     prog->expected_attach_type))
> +		return false;
> +	return bpf_sock_is_valid_access(off, size, type, info);
> +}

This one here should also be swapped to make it more robust, meaning the
__sock_filter_check_attach_type() should come in a second step after basic
sanity checks have been completed, not before them. E.g. out of bounds read
access would then indicate a "good" access in the first one.

^ permalink raw reply

* Re: [PATCHv3 bpf-next 04/12] bpf: Add PTR_TO_SOCKET verifier type
From: Daniel Borkmann @ 2018-09-28 13:29 UTC (permalink / raw)
  To: Joe Stringer
  Cc: netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez
In-Reply-To: <20180927232659.14348-5-joe@wand.net.nz>

Hi Joe,

On 09/28/2018 01:26 AM, Joe Stringer wrote:
> Teach the verifier a little bit about a new type of pointer, a
> PTR_TO_SOCKET. This pointer type is accessed from BPF through the
> 'struct bpf_sock' structure.
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
[...]
>  	}
> @@ -1726,6 +1755,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
>  		err = check_flow_keys_access(env, off, size);
>  		if (!err && t == BPF_READ && value_regno >= 0)
>  			mark_reg_unknown(env, regs, value_regno);
> +	} else if (reg->type == PTR_TO_SOCKET) {
> +		if (t == BPF_WRITE) {
> +			verbose(env, "cannot write into socket\n");
> +			return -EACCES;
> +		}
> +		err = check_sock_access(env, regno, off, size, t);
> +		if (!err && value_regno >= 0)
> +			mark_reg_unknown(env, regs, value_regno);

Not an issue today, but this is quite fragile and very easy to miss, if we
allow to enable writes into ptr_to_socket in future e.g. mark or others,
then lifting above will not be enough. E.g. see check_xadd() and friends,
this rejects writes to ctx via f37a8cb84cce ("bpf: reject stores into ctx
via st and xadd") as otherwise this would bypass the context rewriter. So
I think we should add PTR_TO_SOCKET to is_ctx_reg() as well to have a full
guarantee this won't happen.

>  	} else {
>  		verbose(env, "R%d invalid mem access '%s'\n", regno,
>  			reg_type_str[reg->type]);

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Edward Cree @ 2018-09-28 12:58 UTC (permalink / raw)
  To: Ariel Almog
  Cc: linville, Linux Netdev List, ganeshgr, jakub.kicinski, dustin,
	dirk.vandermerwe, shayag, ariela
In-Reply-To: <CABvr3-Hvq4nsNEfJH+ic_pFGup2iAUGjcDfXni33a6LZoSZOYw@mail.gmail.com>

On 26/09/18 09:47, Ariel Almog wrote:
> I was won
Truncated sentence?  ("... wondering"?)

> I find the ability to set off, auto and specific FEC mode in the same
> command confusing.
I didn't try to define semantics here since each driver currently does
 something slightly different.  Probably the configuration space that's
 meaningful is different for each piece of hardware anyway.

> Here are some examples
>
> 1. What is the expected result of 'off' & other FEC mode such as 'RS'?
>   -'off'?
>   -'RS'?
>   -automatic selection {'off','RS'}? w/o setting of auto?
In sfc, 'off' overrides everything else.

The meaning (again, in sfc) of a combination of 'auto' and a specific mode
 (e.g. 'rs') is "prefer the specified mode, but fall back to autoneg if
 it's not supported".  The combination {'rs', 'baser'} (with or without
 'auto') means "use the strongest FEC supported", i.e. it will attempt to
 negotiate FEC even if the cable & link partner don't request it (e.g. a
 short cable).

For us, those semantics make sense (our HW has a notion of 'supported'
 and 'requested' bits for each FEC type for each of local-device, cable
 and link-partner, and uses the strongest FEC mode that's supported by
 everyone and requested by anyone); but if something else is a better fit
 for your hardware I wouldn't worry too much about the inconsistency —
 people using this functionality will hopefully have read the hardware's
 user manual...

-Ed

^ permalink raw reply

* Fwd: R8169: Network lockups in 4.18.{8,9,10} (and 4.19 dev)
From: Chris Clayton @ 2018-09-28 12:54 UTC (permalink / raw)
  To: netdev
In-Reply-To: <af1b08f1-1280-ff66-4a43-70fa4487838c@googlemail.com>

Forwarding to corrected netdev address.


-------- Forwarded Message --------
Subject: R8169: Network lockups in 4.18.{8,9,10} (and 4.19 dev)
Date: Fri, 28 Sep 2018 13:14:35 +0100
From: Chris Clayton <chris2553@googlemail.com>
To: linux-netdev@vger.kernel.org
CC: David Miller <davem@davemloft.net>, a3at.mail@gmail.com, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
hkallweit1@gmail.com, nic_swsd@realtek.com

Hi,

I upgraded my kernel to 4.18.10 recently and have since been experiencing network problems after resuming from a
suspend to RAM or disk. I previously had 4.18.6 and that was OK.

The pattern of the problem is that when I first boot, the network is fine. But, after resume from suspend I find that
the time taken for a ping of one of my ISP's nameservers increases from 14-15ms to more than 1000ms. Moreover, when I
open a browser (chromium or firefox), it fails to retrieve my home page (https://www.google.co.uk) and pings of the
nameserver fail with the message "Destination Host Unreachable". Often, I can revive the network by stopping it with
/sbin/if(down,up} but sometimes it is necessary to also remove the r8169 module and load it again.

My investigation show that 4.18.7 is also OK, but 4.18.8 has the problem. So I cloned the stable tree and bisected
between 4.18.7 and 4.18.8. The outcome was:

$ git bisect bad
e366979eb8f0c3ad6446abbd81bcb3d4ac569cb3 is the first bad commit
commit e366979eb8f0c3ad6446abbd81bcb3d4ac569cb3
Author: Azat Khuzhin <a3at.mail@gmail.com>
Date:   Sun Aug 26 17:03:09 2018 +0300

    r8169: set RxConfig after tx/rx is enabled for RTL8169sb/8110sb devices

    [ Upstream commit 05212ba8132b42047ab5d63d759c6f9c28e7eab5 ]

    I have two Ethernet adapters:
      r8169 0000:03:01.0 eth0: RTL8169sb/8110sb, 00:14:d1:14:2d:49, XID 10000000, IRQ 18
      r8169 0000:01:00.0 eth0: RTL8168e/8111e, 64:66:b3:11:14:5d, XID 2c200000, IRQ 30
    And after upgrading from linux 4.15 [1] to linux 4.18+ [2] RTL8169sb failed to
    receive any packets. tcpdump shows a lot of checksum mismatch.

      [1]: a0f79386a4968b4925da6db2d1daffd0605a4402
      [2]: 0519359784328bfa92bf0931bf0cff3b58c16932 (4.19 merge window opened)

    I started bisecting and the found that [3] breaks it. According to [4]:
      "For 8110S, 8110SB, and 8110SC series, the initial value of RxConfig
      needs to be set after the tx/rx is enabled."
    So I moved rtl_init_rxcfg() after enabling tx/rs and now my adapter works
    (RTL8168e works too).

      [3]: 3559d81e76bfe3803e89f2e04cf6ef7ab4f3aace
      [4]: e542a2269f232d61270ceddd42b73a4348dee2bb ("r8169: adjust the RxConfig
    settings.")

    Also drop "rx" from rtl_set_rx_tx_config_registers(), since it does nothing
    with it already.

    Fixes: 3559d81e76bfe3803e89f2e04cf6ef7ab4f3aace ("r8169: simplify
    rtl_hw_start_8169")

    Cc: Heiner Kallweit <hkallweit1@gmail.com>
    Cc: David S. Miller <davem@davemloft.net>
    Cc: netdev@vger.kernel.org
    Cc: Realtek linux nic maintainers <nic_swsd@realtek.com>
    Signed-off-by: Azat Khuzhin <a3at.mail@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

:040000 040000 d95481376500f1bc5569fc486bc7573ea201b617 7985ef7b56e340312fe647c7c6e2a24998981304 M	drivers

Ignoring the renaming of a function, the patch added a call to rtl_init_rxcfg() to rtl_hw_start(). If I remove that call
from 4.18.10, my network no longer fails after a resume from suspend.

I should add that I get the same problem with the current 4.19 development kernel, but I haven't tried removing the call
to rtl_init_rxcfg() from the driver. I'll do that later today and report back.

Details of my ethernet device from lspci are:
05:00.2 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
(rev 0a)
	Subsystem: CLEVO/KAPOK Computer RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR- FastB2B- DisINTx-
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 19
	Region 0: I/O ports at e000 [size=256]
	Region 2: Memory at f0004000 (64-bit, prefetchable) [size=4K]
	Region 4: Memory at f0000000 (64-bit, prefetchable) [size=16K]
	Capabilities: [40] Power Management version 3
		Flags: PMEClk- DSI- D1+ D2+ AuxCurrent=375mA PME(D0+,D1+,D2+,D3hot+,D3cold+)
		Status: D0 NoSoftRst+ PME-Enable- DSel=0 DScale=0 PME-
	Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
		Address: 0000000000000000  Data: 0000
	Capabilities: [70] Express (v2) Endpoint, MSI 01
		DevCap:	MaxPayload 128 bytes, PhantFunc 0, Latency L0s <512ns, L1 <64us
			ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset- SlotPowerLimit 10.000W
		DevCtl:	CorrErr- NonFatalErr- FatalErr- UnsupReq-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 4096 bytes
		DevSta:	CorrErr+ NonFatalErr- FatalErr- UnsupReq+ AuxPwr+ TransPend-
		LnkCap:	Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s unlimited, L1 <64us
			ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk+
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s (ok), Width x1 (ok)
			TrErr- Train- SlotClk+ DLActive- BWMgmt- ABWMgmt-
		DevCap2: Completion Timeout: Range ABCD, TimeoutDis+, LTR-, OBFF Not Supported
			 AtomicOpsCap: 32bit- 64bit- 128bitCAS-
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF Disabled
			 AtomicOpsCtl: ReqEn-
		LnkSta2: Current De-emphasis Level: -6dB, EqualizationComplete-, EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
		Vector table: BAR=4 offset=00000000
		PBA: BAR=4 offset=00000800
	Capabilities: [d0] Vital Product Data
pcilib: sysfs_read_vpd: read failed: Input/output error
		Not readable
	Kernel driver in use: r8169
	Kernel modules: r8169

I'm more then happy to provide any additional diagnostics are needed to solve this and to apply diagnostic patches and
proposed fixes.

Thanks

Chris

^ permalink raw reply

* Re: [PATCH net] vxlan: use nla_put_flag for ttl inherit
From: Hangbin Liu @ 2018-09-28 12:38 UTC (permalink / raw)
  To: David Miller; +Cc: Phil Sutter, netdev, Stephen Hemminger, David Ahern
In-Reply-To: <20180928103700.GC14666@orbyte.nwl.cc>

On Fri, Sep 28, 2018 at 12:37:00PM +0200, Phil Sutter wrote:
> On Fri, Sep 28, 2018 at 09:08:26AM +0800, Hangbin Liu wrote:
> > Phil pointed out that there is a mismatch between vxlan and geneve ttl inherit.
> > We should define it as a flag and use nla_put_flag to export this opiton.
> 
> s/opiton/option/
> 
> Apart from that, LGTM!
> 
> Thanks, Phil

Opps, sorry...

Hi David,

Should I re-send a patch or will you help fix it directly?

Thanks
Hangbin

^ permalink raw reply

* Re: [PATCH net-next 00/10] Cleanups, minor additions & fixes for HNS3 driver
From: Eric Dumazet @ 2018-09-28 18:56 UTC (permalink / raw)
  To: David Miller, salil.mehta
  Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
	linuxarm
In-Reply-To: <20180928.103826.486456665561471777.davem@davemloft.net>



On 09/28/2018 10:38 AM, David Miller wrote:
> From: Salil Mehta <salil.mehta@huawei.com>
> Date: Wed, 26 Sep 2018 19:28:30 +0100
> 
>> This patch-set contains cleans-ups, minor changes and fixes to the HNS3 driver.
> 
> Series applied, thank you.
> 

Something seems wrong

# git grep -n HNS3_SELF_TEST_TYPE_NUM
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c:278: int st_param[HNS3_SELF_TEST_TYPE_NUM][2];
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c:316: for (i = 0; i < HNS3_SELF_TEST_TYPE_NUM; i++) {

drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c: In function 'hns3_self_test':
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c:278:15: error: 'HNS3_SELF_TEST_TYPE_NUM' undeclared (first use in this function)
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c:278:15: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c:278:6: error: unused variable 'st_param' [-Werror=unused-variable]

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: permit CGROUP_DEVICE programs accessing helper bpf_get_current_cgroup_id()
From: Daniel Borkmann @ 2018-09-28 12:16 UTC (permalink / raw)
  To: Roman Gushchin, Yonghong Song; +Cc: ast, netdev, kernel-team
In-Reply-To: <20180928095310.GA9018@castle.DHCP.thefacebook.com>

On 09/28/2018 11:53 AM, Roman Gushchin wrote:
> On Thu, Sep 27, 2018 at 02:37:30PM -0700, Yonghong Song wrote:
>> Currently, helper bpf_get_current_cgroup_id() is not permitted
>> for CGROUP_DEVICE type of programs. If the helper is used
>> in such cases, the verifier will log the following error:
>>
>>   0: (bf) r6 = r1
>>   1: (69) r7 = *(u16 *)(r6 +0)
>>   2: (85) call bpf_get_current_cgroup_id#80
>>   unknown func bpf_get_current_cgroup_id#80
>>
>> The bpf_get_current_cgroup_id() is useful for CGROUP_DEVICE
>> type of programs in order to customize action based on cgroup id.
>> This patch added such a support.
>>
>> Cc: Roman Gushchin <guro@fb.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> 
> Acked-by: Roman Gushchin <guro@fb.com>

Applied to bpf-next, thanks!

^ permalink raw reply

* Re: [PATCH bpf-next] bpf: fix flags check in bpf_percpu_cgroup_storage_update()
From: Daniel Borkmann @ 2018-09-28 12:11 UTC (permalink / raw)
  To: Roman Gushchin, netdev; +Cc: linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <20180928110648.22973-1-guro@fb.com>

On 09/28/2018 01:06 PM, Roman Gushchin wrote:
> Fix an issue in bpf_percpu_cgroup_storage_update(): it should return
> -EINVAL on an attempt to pass BPF_NOEXIST rather than BPF_EXIST.
> 
> Cgroup local storage is automatically created on attaching of a bpf
> program to a cgroup, and it can't be done from the userspace.
> 
> Fixes: 0daef9b42374 ("bpf: introduce per-cpu cgroup local storage")
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Alexei Starovoitov <ast@kernel.org>
> ---
>  kernel/bpf/local_storage.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c
> index c739f6dcc3c2..190535f6d5e2 100644
> --- a/kernel/bpf/local_storage.c
> +++ b/kernel/bpf/local_storage.c
> @@ -191,7 +191,7 @@ int bpf_percpu_cgroup_storage_update(struct bpf_map *_map, void *_key,
>  	int cpu, off = 0;
>  	u32 size;
>  
> -	if (unlikely(map_flags & BPF_EXIST))
> +	if (map_flags & BPF_NOEXIST)
>  		return -EINVAL;

Hmm, this is also incorrect as any future reserved flag would be accepted here and
couldn't be extended anymore. :/ And it looks like cgroup_storage_update_elem() is
doing the same today, given the cgroups local storage is still early, we should route
a patch to stable for fixing this.

Wrt this series, given the series is top of tree right now, I would prefer a fresh
respin so we have the fix integrated properly w/o follow-up. Perhaps this could also
incorporate Alexei's previous cleanup suggestions as well from today if you have a
chance.

Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH] qed: fix spelling mistake "b_cb_registred" -> "b_cb_registered"
From: David Miller @ 2018-09-28 18:15 UTC (permalink / raw)
  To: colin.king
  Cc: Ariel.Elior, everest-linux-l2, netdev, kernel-janitors,
	linux-kernel
In-Reply-To: <20180927170454.7829-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Thu, 27 Sep 2018 18:04:54 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake struct field name, rename it.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied to net-next, thanks Colin.

^ permalink raw reply

* Re: [Patch net-next] net_sched: fix an extack message in tcf_block_find()
From: Vlad Buslov @ 2018-09-28 11:36 UTC (permalink / raw)
  To: Cong Wang; +Cc: netdev, jiri, jhs
In-Reply-To: <20180927204219.17846-1-xiyou.wangcong@gmail.com>

On Thu 27 Sep 2018 at 20:42, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> It is clearly a copy-n-paste.
>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
>  net/sched/cls_api.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 3de47e99b788..8dd7f8af6d54 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -655,7 +655,7 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
>  
>  		*q = qdisc_refcount_inc_nz(*q);
>  		if (!*q) {
> -			NL_SET_ERR_MSG(extack, "Parent Qdisc doesn't exists");
> +			NL_SET_ERR_MSG(extack, "Can't increase Qdisc refcount");
>  			err = -EINVAL;
>  			goto errout_rcu;
>  		}

Is there a benefit in exposing this info to user?
For all intents and purposes Qdisc is gone at that point.

^ permalink raw reply

* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Ard Biesheuvel @ 2018-09-28 17:47 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Eric Biggers, LKML, Netdev, Linux Crypto Mailing List,
	David Miller, Greg Kroah-Hartman
In-Reply-To: <CAHmME9rJ4CewfDEqWh00ZowjWg9TaYpksVSzKSSxoKya-M8_LA@mail.gmail.com>

On 28 September 2018 at 07:46, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi Eric,
>
> On Fri, Sep 28, 2018 at 6:55 AM Eric Biggers <ebiggers@kernel.org> wrote:
>> And you still haven't answered my question about adding a new algorithm that is
>> useful to have in both APIs.  You're proposing that in most cases the crypto API
>> part will need to go through Herbert while the implementation will need to go
>> through you/Greg, right?  Or will you/Greg be taking both?  Or will Herbert take
>> both?
>
> If an implementation enters Zinc, it will go through my tree. If it
> enters the crypto API, it will go through Herbert's tree. If there
> wind up being messy tree dependency and merge timing issues, I can
> work this out in the usual way with Herbert. I'll be sure to discuss
> these edge cases with him when we discuss. I think there's a way to
> handle that with minimum friction.
>
>> A documentation file for Zinc is a good idea.  Some of the information in your
>> commit messages should be moved there too.
>
> Will do.
>
>> I'm not trying to "politicize" this, but rather determine your criteria for
>> including algorithms in Zinc, which you haven't made clear.  Since for the
>> second time you've avoided answering my question about whether you'd allow the
>> SM4 cipher in Zinc, and you designed WireGuard to be "cryptographically
>> opinionated", and you were one of the loudest voices calling for the Speck
>> cipher to be removed, and your justification for Zinc complains about cipher
>> modes from "90s cryptographers", I think it's reasonable for people to wonder
>> whether as the Zinc (i.e. Linux crypto library) maintainer you will restrict the
>> inclusion of crypto algorithms to the ones you prefer, rather than the ones that
>> users need.  Note that the kernel is used by people all over the world and needs
>> to support various standards, protocols, and APIs that use different crypto
>> algorithms, not always the ones we'd like; and different users have different
>> preferences.  People need to know whether the Linux kernel's crypto library will
>> meet (or be allowed to meet) their crypto needs.
>
> WireGuard is indeed quite opinionated in its primitive choices, but I
> don't think it'd be wise to apply the same design to Zinc. There are
> APIs where the goal is to have a limited set of high-level functions,
> and have those implemented in only a preferred set of primitives. NaCl
> is a good example of this -- functions like "crypto_secretbox" that
> are actually xsalsapoly under the hood. Zinc doesn't intend to become
> an API like those, but rather to provide the actual primitives for use
> cases where that specific primitive is used. Sometimes the kernel is
> in the business of being able to pick its own crypto -- random.c, tcp
> sequence numbers, big_key.c, etc -- but most of the time the kernel is
> in the business of implementing other people's crypto, for specific
> devices/protocols/diskformats. And for those use cases we need not
> some high-level API like NaCl, but rather direct access to the
> primitives that are required to implement those drivers. WireGuard is
> one such example, but so would be Bluetooth, CIFS/SMB, WiFi, and so
> on. We're in the business of writing drivers, after all. So, no, I
> don't think I'd knock down the addition of a primitive because of a
> simple preference for a different primitive, if it was clearly the
> case that the driver requiring it really benefited from having
> accessible via the plain Zinc function calls. Sorry if I hadn't made
> this clear earlier -- I thought Ard had asked more or less the same
> thing about DES and I answered accordingly, but maybe that wasn't made
> clear enough there.
>

CRC32 is another case study that I would like to bring up:
- the current status is a bit of a mess, where we treat crc32() and
crc32c() differently - the latter is exposed by libcrc32.ko as a
wrapper around the crypto API, and there is some networking code (SCTP
iirc) that puts another kludge on top of that to ensure that the code
is not used before the module is loaded
- we have C versions, scalar hw instruction based versions and
carryless multiplication based versions for various architectures
- on ST platforms, we have a synchronous hw accelerator that is 10x
faster than C code on the same platform.

AFAICT none of its current users rely on the async interface or
runtime dispatch of the 'hash'.

CRC32/c is an area that I would *really* like to clean up (and am
already doing for arm64) - just having a function call interface would
be a huge improvement but it seems to me that the choice for
monolithic modules per algo/architecture implies that we will have to
leave ST behind in this case.

On the one hand, disregarding this seems fair, at least for the
moment. On the other hand, fixing this in the crypto API, e.g., by
permitting direct function calls to synchronous hashes and without the
need to allocate a transform/request etc, blurs the line even more
where different pieces should live.

Any thoughts?

^ permalink raw reply

* Re: [PATCH net-next 00/10] Cleanups, minor additions & fixes for HNS3 driver
From: David Miller @ 2018-09-28 17:38 UTC (permalink / raw)
  To: salil.mehta
  Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
	linuxarm
In-Reply-To: <20180926182840.28392-1-salil.mehta@huawei.com>

From: Salil Mehta <salil.mehta@huawei.com>
Date: Wed, 26 Sep 2018 19:28:30 +0100

> This patch-set contains cleans-ups, minor changes and fixes to the HNS3 driver.

Series applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: b53: Fix build with B53_SRAB enabled and B53_SERDES=m
From: David Miller @ 2018-09-28 17:35 UTC (permalink / raw)
  To: f.fainelli; +Cc: arnd, andrew, vivien.didelot, netdev, linux-kernel
In-Reply-To: <c0af4a4c-6656-e014-a4bb-0bacfbe1d468@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 27 Sep 2018 12:07:01 -0700

> On 09/27/2018 03:02 AM, Arnd Bergmann wrote:
>> When B53_SERDES is a loadable module, a built-in srab driver still
>> cannot reach it, so the previous fix is incomplete:
>> 
>> b53_srab.c:(.text+0x3f4): undefined reference to `b53_serdes_init'
>> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe64): undefined reference to `b53_serdes_link_state'
>> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe74): undefined reference to `b53_serdes_link_set'
>> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe88): undefined reference to `b53_serdes_an_restart'
>> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea0): undefined reference to `b53_serdes_phylink_validate'
>> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea4): undefined reference to `b53_serdes_config'
>> 
>> Add a Kconfig dependency that forces srab to also be a module
>> in this case, but allow it to be built-in when serdes is
>> disabled or built-in.
>> 
>> Fixes: 7a8c7f5c30f9 ("net: dsa: b53: Fix build with B53_SRAB enabled and not B53_SERDES")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Acked-by: Florian Fainelli <f.fainelli@gmail.com>

Applied, thanks Arnd.

^ permalink raw reply

* Re: [PATCH net-next] net: dsa: b53: Fix build with B53_SRAB enabled and B53_SERDES=m
From: David Miller @ 2018-09-28 17:33 UTC (permalink / raw)
  To: arnd; +Cc: f.fainelli, andrew, vivien.didelot, netdev, linux-kernel
In-Reply-To: <20180927100244.692546-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Thu, 27 Sep 2018 12:02:38 +0200

> When B53_SERDES is a loadable module, a built-in srab driver still
> cannot reach it, so the previous fix is incomplete:
> 
> b53_srab.c:(.text+0x3f4): undefined reference to `b53_serdes_init'
> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe64): undefined reference to `b53_serdes_link_state'
> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe74): undefined reference to `b53_serdes_link_set'
> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xe88): undefined reference to `b53_serdes_an_restart'
> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea0): undefined reference to `b53_serdes_phylink_validate'
> drivers/net/dsa/b53/b53_srab.o:(.rodata+0xea4): undefined reference to `b53_serdes_config'
> 
> Add a Kconfig dependency that forces srab to also be a module
> in this case, but allow it to be built-in when serdes is
> disabled or built-in.
> 
> Fixes: 7a8c7f5c30f9 ("net: dsa: b53: Fix build with B53_SRAB enabled and not B53_SERDES")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied, thanks Arnd.

^ permalink raw reply

* Re: [PATCH RFC net-next] openvswitch: Queue upcalls to userspace in per-port round-robin order
From: Pravin Shelar @ 2018-09-28 17:15 UTC (permalink / raw)
  To: Stefano Brivio
  Cc: ovs dev, Justin Pettit, Linux Kernel Network Developers,
	Jiri Benc
In-Reply-To: <20180926115821.7cd9fed8@epycfail>

On Wed, Sep 26, 2018 at 2:58 AM Stefano Brivio <sbrivio-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> Hi Pravin,
>
> On Wed, 15 Aug 2018 00:19:39 -0700
> Pravin Shelar <pshelar-LZ6Gd1LRuIk@public.gmane.org> wrote:
>
> > I understand fairness has cost, but we need to find right balance
> > between performance and fairness. Current fairness scheme is a
> > lockless algorithm without much computational overhead, did you try to
> > improve current algorithm so that it uses less number of ports.
>
> In the end, we tried harder as you suggested, and found out a way to
> avoid the need for per-thread sets of per-vport sockets: with a few
> changes, a process-wide set of per-vport sockets is actually enough to
> maintain the current fairness behaviour.
>
> Further, we can get rid of duplicate fd events if the EPOLLEXCLUSIVE
> epoll() flag is available, which improves performance noticeably. This
> is explained in more detail in the commit message of the ovs-vswitchd
> patch Matteo wrote [1], now merged.
>
> This approach solves the biggest issue (i.e. huge amount of netlink
> sockets) in ovs-vswitchd itself, without the need for kernel changes.
>

Thanks for working on this issue.

> It doesn't address some proposed improvements though, that is, it does
> nothing to improve the current fairness scheme, it won't allow neither
> the configurable fairness criteria proposed by Ben, nor usage of BPF
> maps for extensibility as suggested by William.
>
> I would however say that those topics bear definitely lower priority,
> and perhaps addressing them at all becomes overkill now that we don't
> any longer have a serious issue.
>
> [1] https://patchwork.ozlabs.org/patch/974304/
Nice!

Thanks,
Pravin.

^ permalink raw reply

* Re: INFO: trying to register non-static key in tun_chr_write_iter
From: Eric Dumazet @ 2018-09-28 17:06 UTC (permalink / raw)
  To: syzbot, brouer, davem, edumazet, jasowang, linux-kernel, mst,
	netdev, sd, syzkaller-bugs
In-Reply-To: <000000000000c1119b0576efc7f6@google.com>



On 09/28/2018 08:05 AM, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    100811936f89 bpf: test_bpf: add init_net to dev for flow_d..
> git tree:       bpf-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=145378e6400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=443816db871edd66
> dashboard link: https://syzkaller.appspot.com/bug?extid=e662df0ac1d753b57e80
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
>


I took a look at this report, I will send a patch series fixing the issues
I found in tun driver.

^ permalink raw reply

* Re: [PATCH net-next] geneve: fix ttl inherit type
From: Phil Sutter @ 2018-09-28 10:38 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, David Miller, Stephen Hemminger, David Ahern
In-Reply-To: <1538096998-20937-1-git-send-email-liuhangbin@gmail.com>

On Fri, Sep 28, 2018 at 09:09:58AM +0800, Hangbin Liu wrote:
> Phil pointed out that there is a mismatch between vxlan and geneve ttl
> inherit. We should define it as a flag and use nla_put_flag to export this
> opiton.

Same typo here. :)

Apart from that, LGTM!

Thanks, Phil

^ permalink raw reply

* Re: [PATCH net] vxlan: use nla_put_flag for ttl inherit
From: Phil Sutter @ 2018-09-28 10:37 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, David Miller, Stephen Hemminger, David Ahern
In-Reply-To: <1538096906-20866-1-git-send-email-liuhangbin@gmail.com>

On Fri, Sep 28, 2018 at 09:08:26AM +0800, Hangbin Liu wrote:
> Phil pointed out that there is a mismatch between vxlan and geneve ttl inherit.
> We should define it as a flag and use nla_put_flag to export this opiton.

s/opiton/option/

Apart from that, LGTM!

Thanks, Phil

^ permalink raw reply

* pull-request: ieee802154 for net 2018-09-28
From: Stefan Schmidt @ 2018-09-28 10:20 UTC (permalink / raw)
  To: davem; +Cc: linux-wpan, alex.aring, netdev

Hello Dave.

An update from ieee802154 for your *net* tree.

Some cleanup patches throughout the drivers from the Huawei tag team
Yue Haibing and Zhong Jiang.
Xue is replacing some magic numbers with defines in his mcr20a driver.

regards
Stefan Schmidt

The following changes since commit 10bc6a6042c900934a097988b5d50e3cf3f91781:

  r8169: fix autoneg issue on resume with RTL8168E (2018-09-20 19:58:47 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/sschmidt/wpan.git ieee802154-for-davem-2018-09-28

for you to fetch changes up to d6d1cd2578c4da0764ad334e3411c1c1b1557f58:

  ieee802154: mcr20a: Replace magic number with constants (2018-09-27 17:22:48 +0200)

----------------------------------------------------------------
Xue Liu (1):
      ieee802154: mcr20a: Replace magic number with constants

YueHaibing (1):
      ieee802154: Use kmemdup instead of duplicating it in ca8210_test_int_driver_write

zhong jiang (2):
      ieee802154: remove unecessary condition check before debugfs_remove_recursive
      ieee802154: ca8210: remove redundant condition check before debugfs_remove

 drivers/net/ieee802154/adf7242.c | 3 +--
 drivers/net/ieee802154/ca8210.c  | 6 ++----
 drivers/net/ieee802154/mcr20a.c  | 8 ++++----
 3 files changed, 7 insertions(+), 10 deletions(-)

^ permalink raw reply

* [PATCH net 4/8] rxrpc: Emit BUSY packets when supposed to rather than ABORTs
From: David Howells @ 2018-09-28 10:10 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <153812939111.10469.179989653628053410.stgit@warthog.procyon.org.uk>

In the input path, a received sk_buff can be marked for rejection by
setting RXRPC_SKB_MARK_* in skb->mark and, if needed, some auxiliary data
(such as an abort code) in skb->priority.  The rejection is handled by
queueing the sk_buff up for dealing with in process context.  The output
code reads the mark and priority and, theoretically, generates an
appropriate response packet.

However, if RXRPC_SKB_MARK_BUSY is set, this isn't noticed and an ABORT
message with a random abort code is generated (since skb->priority wasn't
set to anything).

Fix this by outputting the appropriate sort of packet.

Also, whilst we're at it, most of the marks are no longer used, so remove
them and rename the remaining two to something more obvious.

Fixes: 248f219cb8bc ("rxrpc: Rewrite the data and ack handling code")
Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |   13 ++++---------
 net/rxrpc/call_accept.c |    6 +++---
 net/rxrpc/input.c       |    2 +-
 net/rxrpc/output.c      |   23 ++++++++++++++++++-----
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 9fcb3e197b14..e8861cb78070 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -40,17 +40,12 @@ struct rxrpc_crypt {
 struct rxrpc_connection;
 
 /*
- * Mark applied to socket buffers.
+ * Mark applied to socket buffers in skb->mark.  skb->priority is used
+ * to pass supplementary information.
  */
 enum rxrpc_skb_mark {
-	RXRPC_SKB_MARK_DATA,		/* data message */
-	RXRPC_SKB_MARK_FINAL_ACK,	/* final ACK received message */
-	RXRPC_SKB_MARK_BUSY,		/* server busy message */
-	RXRPC_SKB_MARK_REMOTE_ABORT,	/* remote abort message */
-	RXRPC_SKB_MARK_LOCAL_ABORT,	/* local abort message */
-	RXRPC_SKB_MARK_NET_ERROR,	/* network error message */
-	RXRPC_SKB_MARK_LOCAL_ERROR,	/* local error message */
-	RXRPC_SKB_MARK_NEW_CALL,	/* local error message */
+	RXRPC_SKB_MARK_REJECT_BUSY,	/* Reject with BUSY */
+	RXRPC_SKB_MARK_REJECT_ABORT,	/* Reject with ABORT (code in skb->priority) */
 };
 
 /*
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 9d1e298b784c..e88f131c1d7f 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -353,7 +353,7 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 
 	trace_rxrpc_abort(0, "INV", sp->hdr.cid, sp->hdr.callNumber, sp->hdr.seq,
 			  RX_INVALID_OPERATION, EOPNOTSUPP);
-	skb->mark = RXRPC_SKB_MARK_LOCAL_ABORT;
+	skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
 	skb->priority = RX_INVALID_OPERATION;
 	_leave(" = NULL [service]");
 	return NULL;
@@ -364,7 +364,7 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 	    rx->sk.sk_state == RXRPC_CLOSE) {
 		trace_rxrpc_abort(0, "CLS", sp->hdr.cid, sp->hdr.callNumber,
 				  sp->hdr.seq, RX_INVALID_OPERATION, ESHUTDOWN);
-		skb->mark = RXRPC_SKB_MARK_LOCAL_ABORT;
+		skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
 		skb->priority = RX_INVALID_OPERATION;
 		_leave(" = NULL [close]");
 		call = NULL;
@@ -373,7 +373,7 @@ struct rxrpc_call *rxrpc_new_incoming_call(struct rxrpc_local *local,
 
 	call = rxrpc_alloc_incoming_call(rx, local, conn, skb);
 	if (!call) {
-		skb->mark = RXRPC_SKB_MARK_BUSY;
+		skb->mark = RXRPC_SKB_MARK_REJECT_BUSY;
 		_leave(" = NULL [busy]");
 		call = NULL;
 		goto out;
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 7f9ed3a60b9a..b0f12471f5e7 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -1354,7 +1354,7 @@ void rxrpc_data_ready(struct sock *udp_sk)
 protocol_error:
 	skb->priority = RX_PROTOCOL_ERROR;
 post_abort:
-	skb->mark = RXRPC_SKB_MARK_LOCAL_ABORT;
+	skb->mark = RXRPC_SKB_MARK_REJECT_ABORT;
 reject_packet:
 	trace_rxrpc_rx_done(skb->mark, skb->priority);
 	rxrpc_reject_packet(local, skb);
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index 8a4da3fe96df..e8fb8922bca8 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -524,7 +524,7 @@ void rxrpc_reject_packets(struct rxrpc_local *local)
 	struct kvec iov[2];
 	size_t size;
 	__be32 code;
-	int ret;
+	int ret, ioc;
 
 	_enter("%d", local->debug_id);
 
@@ -532,7 +532,6 @@ void rxrpc_reject_packets(struct rxrpc_local *local)
 	iov[0].iov_len = sizeof(whdr);
 	iov[1].iov_base = &code;
 	iov[1].iov_len = sizeof(code);
-	size = sizeof(whdr) + sizeof(code);
 
 	msg.msg_name = &srx.transport;
 	msg.msg_control = NULL;
@@ -540,17 +539,31 @@ void rxrpc_reject_packets(struct rxrpc_local *local)
 	msg.msg_flags = 0;
 
 	memset(&whdr, 0, sizeof(whdr));
-	whdr.type = RXRPC_PACKET_TYPE_ABORT;
 
 	while ((skb = skb_dequeue(&local->reject_queue))) {
 		rxrpc_see_skb(skb, rxrpc_skb_rx_seen);
 		sp = rxrpc_skb(skb);
 
+		switch (skb->mark) {
+		case RXRPC_SKB_MARK_REJECT_BUSY:
+			whdr.type = RXRPC_PACKET_TYPE_BUSY;
+			size = sizeof(whdr);
+			ioc = 1;
+			break;
+		case RXRPC_SKB_MARK_REJECT_ABORT:
+			whdr.type = RXRPC_PACKET_TYPE_ABORT;
+			code = htonl(skb->priority);
+			size = sizeof(whdr) + sizeof(code);
+			ioc = 2;
+			break;
+		default:
+			rxrpc_free_skb(skb, rxrpc_skb_rx_freed);
+			continue;
+		}
+
 		if (rxrpc_extract_addr_from_skb(local, &srx, skb) == 0) {
 			msg.msg_namelen = srx.transport_len;
 
-			code = htonl(skb->priority);
-
 			whdr.epoch	= htonl(sp->hdr.epoch);
 			whdr.cid	= htonl(sp->hdr.cid);
 			whdr.callNumber	= htonl(sp->hdr.callNumber);

^ permalink raw reply related

* Re: [PATCH 13/15] octeontx2-af: Add support for CGX link management
From: Arnd Bergmann @ 2018-09-28 10:00 UTC (permalink / raw)
  To: Sunil Kovvuri; +Cc: Networking, David Miller, linux-soc, lcherian, nmani
In-Reply-To: <CA+sq2Cc5p+Hdm573is2rAWMeGZKS+=iUiYHU3+soTR4CSpgcHg@mail.gmail.com>

On Fri, Sep 28, 2018 at 11:31 AM Sunil Kovvuri <sunil.kovvuri@gmail.com> wrote:
> On Fri, Sep 28, 2018 at 1:49 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Sep 28, 2018 at 8:09 AM <sunil.kovvuri@gmail.com> wrote:

> This communication between firmware and kernel driver is done using couple of
> scratch registers. With limited space available we had to resort to bitfields.
> Your point about endianness is correct. As you might be aware that the device to
> which this driver registers to, is only found on OcteonTx2 SOC which operates
> in a standalone mode. As of now we are not targeting to make these drivers
> work in big-endian mode.
>
> We would prefer to make big-endian related changes later on, test them
> fully and
> submit patches,  would this be okay ?
>
> If not we will define big endian bit fields in all command structures
> and re-submit.

Generally speaking, I think all drivers should be written in a portable way,
since you never know whether they will be reused in a different way later,
copied into other drivers, or used in creative ways by your users.

I would therefore recommend to change the bitfields now, but not
necessarily test big-endian builds. Well-written code should just work
out of the box in either endianess, and if someone finds a problem
there later, they can fix it themselves or report it. Just don't use
nonportable code intentionally.

        Arnd

^ permalink raw reply

* [PATCH net-next] selftests/tls: Fix recv(MSG_PEEK) & splice() test cases
From: Vakul Garg @ 2018-09-28 16:18 UTC (permalink / raw)
  To: netdev
  Cc: linux-kselftest, linux-kernel, shuah, davejwatson, davem, doronrk,
	Vakul Garg

TLS test cases splice_from_pipe, send_and_splice &
recv_peek_multiple_records expect to receive a given nummber of bytes
and then compare them against the number of bytes which were sent.
Therefore, system call recv() must not return before receiving the
requested number of bytes, otherwise the subsequent memcmp() fails.
This patch passes MSG_WAITALL flag to recv() so that it does not return
prematurely before requested number of bytes are copied to receive
buffer.

Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
 tools/testing/selftests/net/tls.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index 11d54c36ae49..fac68d710f35 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -288,7 +288,7 @@ TEST_F(tls, splice_from_pipe)
 	ASSERT_GE(pipe(p), 0);
 	EXPECT_GE(write(p[1], mem_send, send_len), 0);
 	EXPECT_GE(splice(p[0], NULL, self->fd, NULL, send_len, 0), 0);
-	EXPECT_GE(recv(self->cfd, mem_recv, send_len, 0), 0);
+	EXPECT_EQ(recv(self->cfd, mem_recv, send_len, MSG_WAITALL), send_len);
 	EXPECT_EQ(memcmp(mem_send, mem_recv, send_len), 0);
 }
 
@@ -322,13 +322,13 @@ TEST_F(tls, send_and_splice)
 
 	ASSERT_GE(pipe(p), 0);
 	EXPECT_EQ(send(self->fd, test_str, send_len2, 0), send_len2);
-	EXPECT_NE(recv(self->cfd, buf, send_len2, 0), -1);
+	EXPECT_EQ(recv(self->cfd, buf, send_len2, MSG_WAITALL), send_len2);
 	EXPECT_EQ(memcmp(test_str, buf, send_len2), 0);
 
 	EXPECT_GE(write(p[1], mem_send, send_len), send_len);
 	EXPECT_GE(splice(p[0], NULL, self->fd, NULL, send_len, 0), send_len);
 
-	EXPECT_GE(recv(self->cfd, mem_recv, send_len, 0), 0);
+	EXPECT_EQ(recv(self->cfd, mem_recv, send_len, MSG_WAITALL), send_len);
 	EXPECT_EQ(memcmp(mem_send, mem_recv, send_len), 0);
 }
 
@@ -516,17 +516,17 @@ TEST_F(tls, recv_peek_multiple_records)
 	len = strlen(test_str_second) + 1;
 	EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
 
-	len = sizeof(buf);
+	len = strlen(test_str_first);
 	memset(buf, 0, len);
-	EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+	EXPECT_EQ(recv(self->cfd, buf, len, MSG_PEEK | MSG_WAITALL), len);
 
 	/* MSG_PEEK can only peek into the current record. */
-	len = strlen(test_str_first) + 1;
+	len = strlen(test_str_first);
 	EXPECT_EQ(memcmp(test_str_first, buf, len), 0);
 
-	len = sizeof(buf);
+	len = strlen(test_str) + 1;
 	memset(buf, 0, len);
-	EXPECT_NE(recv(self->cfd, buf, len, 0), -1);
+	EXPECT_EQ(recv(self->cfd, buf, len, MSG_WAITALL), len);
 
 	/* Non-MSG_PEEK will advance strparser (and therefore record)
 	 * however.
@@ -543,9 +543,9 @@ TEST_F(tls, recv_peek_multiple_records)
 	len = strlen(test_str_second) + 1;
 	EXPECT_EQ(send(self->fd, test_str_second, len, 0), len);
 
-	len = sizeof(buf);
+	len = strlen(test_str) + 1;
 	memset(buf, 0, len);
-	EXPECT_NE(recv(self->cfd, buf, len, MSG_PEEK), -1);
+	EXPECT_EQ(recv(self->cfd, buf, len, MSG_PEEK | MSG_WAITALL), len);
 
 	len = strlen(test_str) + 1;
 	EXPECT_EQ(memcmp(test_str, buf, len), 0);
-- 
2.13.6

^ permalink raw reply related

* Re: [PATCH bpf-next] bpf: permit CGROUP_DEVICE programs accessing helper bpf_get_current_cgroup_id()
From: Roman Gushchin @ 2018-09-28  9:53 UTC (permalink / raw)
  To: Yonghong Song; +Cc: ast, daniel, netdev, kernel-team
In-Reply-To: <20180927213730.1224816-1-yhs@fb.com>

On Thu, Sep 27, 2018 at 02:37:30PM -0700, Yonghong Song wrote:
> Currently, helper bpf_get_current_cgroup_id() is not permitted
> for CGROUP_DEVICE type of programs. If the helper is used
> in such cases, the verifier will log the following error:
> 
>   0: (bf) r6 = r1
>   1: (69) r7 = *(u16 *)(r6 +0)
>   2: (85) call bpf_get_current_cgroup_id#80
>   unknown func bpf_get_current_cgroup_id#80
> 
> The bpf_get_current_cgroup_id() is useful for CGROUP_DEVICE
> type of programs in order to customize action based on cgroup id.
> This patch added such a support.
> 
> Cc: Roman Gushchin <guro@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>

Acked-by: Roman Gushchin <guro@fb.com>

Thanks, Yonghong!

^ permalink raw reply

* Re: [PATCHv3 bpf-next 07/12] bpf: Add helper to retrieve socket in BPF
From: Alexei Starovoitov @ 2018-09-28  9:46 UTC (permalink / raw)
  To: Joe Stringer
  Cc: daniel, netdev, ast, john.fastabend, tgraf, kafai, nitin.hande,
	mauricio.vasquez
In-Reply-To: <20180927232659.14348-8-joe@wand.net.nz>

On Thu, Sep 27, 2018 at 04:26:54PM -0700, Joe Stringer wrote:
> This patch adds new BPF helper functions, bpf_sk_lookup_tcp() and
> bpf_sk_lookup_udp() which allows BPF programs to find out if there is a
> socket listening on this host, and returns a socket pointer which the
> BPF program can then access to determine, for instance, whether to
> forward or drop traffic. bpf_sk_lookup_xxx() may take a reference on the
> socket, so when a BPF program makes use of this function, it must
> subsequently pass the returned pointer into the newly added sk_release()
> to return the reference.
> 
> By way of example, the following pseudocode would filter inbound
> connections at XDP if there is no corresponding service listening for
> the traffic:
> 
>   struct bpf_sock_tuple tuple;
>   struct bpf_sock_ops *sk;
> 
>   populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
>   sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof tuple, netns, 0);
>   if (!sk) {
>     // Couldn't find a socket listening for this traffic. Drop.
>     return TC_ACT_SHOT;
>   }
>   bpf_sk_release(sk);
>   return TC_ACT_OK;
> 
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> ---
> v2: Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
>     Limit netns_id field to 32 bits
>     Fix compile error with CONFIG_IPV6 enabled
>     Allow direct packet access from helper
> 
> v3: Fix release of caller_net when netns is not specified.
>     Use skb->sk to find caller net when skb->dev is unavailable.
>     Remove flags argument to sk_release()
>     Define the semantics of the new helpers more clearly.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ 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