* Re: [PATCH bpf-next v5 0/3] xdp: Allow lookup into devmaps before redirect
From: Toke Høiland-Jørgensen @ 2019-06-24 19:38 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Networking, Jesper Dangaard Brouer, Daniel Borkmann,
Alexei Starovoitov, David Miller, Jonathan Lemon
In-Reply-To: <CAEf4BzYFCAp7yUU80ia=C5ywDBgepeaMmVPJW8VG4gLUT=ht=A@mail.gmail.com>
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 :)
-Toke
^ permalink raw reply
* Re: [PATCH 03/11] xdp: add offset param to zero_copy_allocator
From: Jakub Kicinski @ 2019-06-24 19:23 UTC (permalink / raw)
To: Kevin Laatz
Cc: netdev, ast, daniel, bjorn.topel, magnus.karlsson, bpf,
intel-wired-lan, bruce.richardson, ciara.loftus
In-Reply-To: <20190620090958.2135-4-kevin.laatz@intel.com>
On Thu, 20 Jun 2019 09:09:50 +0000, Kevin Laatz wrote:
> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index 0f25b3675c5c..ea801fd2bf98 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -53,7 +53,8 @@ struct xdp_mem_info {
> struct page_pool;
>
> struct zero_copy_allocator {
> - void (*free)(struct zero_copy_allocator *zca, unsigned long handle);
> + void (*free)(struct zero_copy_allocator *zca, unsigned long handle,
> + off_t off);
> };
Please run checkpatch --strict on all your changes. The code
formatting is incorrect in many ways in this series.
Please include performance measurements proving the slow down
is negligible in the cover letter.
^ permalink raw reply
* Re: [PATCH V2 03/15] ARM: ep93xx: cleanup cppcheck shifting errors
From: Alexander Sverdlin @ 2019-06-24 19:16 UTC (permalink / raw)
To: Phong Tran
Cc: acme, alexander.shishkin, allison, andrew, ast, bgolaszewski, bpf,
daniel, daniel, dmg, festevam, gerg, gregkh, gregory.clement,
haojian.zhuang, hsweeten, illusionist.neo, info, jason, jolsa,
kafai, kernel, kgene, krzk, kstewart, linux-arm-kernel, linux-imx,
linux-kernel, linux-omap, linux-samsung-soc, linux, liviu.dudau,
lkundrak, lorenzo.pieralisi, mark.rutland, mingo, namhyung,
netdev, nsekhar, peterz, robert.jarzmik, s.hauer,
sebastian.hesselbarth, shawnguo, songliubraving, sudeep.holla,
swinslow, tglx, tony, will, yhs
In-Reply-To: <20190624135105.15579-4-tranmanphong@gmail.com>
Hi!
On 24/06/2019 15:50, Phong Tran wrote:
> [arch/arm/mach-ep93xx/clock.c:102]: (error) Shifting signed 32-bit value
> by 31 bits is undefined behaviour
> [arch/arm/mach-ep93xx/clock.c:132]: (error) Shifting signed 32-bit value
> by 31 bits is undefined behaviour
> [arch/arm/mach-ep93xx/clock.c:140]: (error) Shifting signed 32-bit value
> by 31 bits is undefined behaviour
> [arch/arm/mach-ep93xx/core.c:1001]: (error) Shifting signed 32-bit value
> by 31 bits is undefined behaviour
> [arch/arm/mach-ep93xx/core.c:1002]: (error) Shifting signed 32-bit value
> by 31 bits is undefined behaviour
Acked-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> Signed-off-by: Phong Tran <tranmanphong@gmail.com>
> ---
> arch/arm/mach-ep93xx/soc.h | 132 ++++++++++++++++++++++-----------------------
> 1 file changed, 66 insertions(+), 66 deletions(-)
>
> diff --git a/arch/arm/mach-ep93xx/soc.h b/arch/arm/mach-ep93xx/soc.h
> index f2dace1c9154..250c82f8b0a1 100644
> --- a/arch/arm/mach-ep93xx/soc.h
> +++ b/arch/arm/mach-ep93xx/soc.h
> @@ -109,89 +109,89 @@
> #define EP93XX_SYSCON_REG(x) (EP93XX_SYSCON_BASE + (x))
> #define EP93XX_SYSCON_POWER_STATE EP93XX_SYSCON_REG(0x00)
> #define EP93XX_SYSCON_PWRCNT EP93XX_SYSCON_REG(0x04)
> -#define EP93XX_SYSCON_PWRCNT_FIR_EN (1<<31)
> -#define EP93XX_SYSCON_PWRCNT_UARTBAUD (1<<29)
> -#define EP93XX_SYSCON_PWRCNT_USH_EN (1<<28)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2M1 (1<<27)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2M0 (1<<26)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P8 (1<<25)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P9 (1<<24)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P6 (1<<23)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P7 (1<<22)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P4 (1<<21)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P5 (1<<20)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P2 (1<<19)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P3 (1<<18)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P0 (1<<17)
> -#define EP93XX_SYSCON_PWRCNT_DMA_M2P1 (1<<16)
> +#define EP93XX_SYSCON_PWRCNT_FIR_EN BIT(31)
> +#define EP93XX_SYSCON_PWRCNT_UARTBAUD BIT(29)
> +#define EP93XX_SYSCON_PWRCNT_USH_EN BIT(28)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M1 BIT(27)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2M0 BIT(26)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P8 BIT(25)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P9 BIT(24)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P6 BIT(23)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P7 BIT(22)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P4 BIT(21)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P5 BIT(20)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P2 BIT(19)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P3 BIT(18)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P0 BIT(17)
> +#define EP93XX_SYSCON_PWRCNT_DMA_M2P1 BIT(16)
> #define EP93XX_SYSCON_HALT EP93XX_SYSCON_REG(0x08)
> #define EP93XX_SYSCON_STANDBY EP93XX_SYSCON_REG(0x0c)
> #define EP93XX_SYSCON_CLKSET1 EP93XX_SYSCON_REG(0x20)
> -#define EP93XX_SYSCON_CLKSET1_NBYP1 (1<<23)
> +#define EP93XX_SYSCON_CLKSET1_NBYP1 BIT(23)
> #define EP93XX_SYSCON_CLKSET2 EP93XX_SYSCON_REG(0x24)
> -#define EP93XX_SYSCON_CLKSET2_NBYP2 (1<<19)
> -#define EP93XX_SYSCON_CLKSET2_PLL2_EN (1<<18)
> +#define EP93XX_SYSCON_CLKSET2_NBYP2 BIT(19)
> +#define EP93XX_SYSCON_CLKSET2_PLL2_EN BIT(18)
> #define EP93XX_SYSCON_DEVCFG EP93XX_SYSCON_REG(0x80)
> -#define EP93XX_SYSCON_DEVCFG_SWRST (1<<31)
> -#define EP93XX_SYSCON_DEVCFG_D1ONG (1<<30)
> -#define EP93XX_SYSCON_DEVCFG_D0ONG (1<<29)
> -#define EP93XX_SYSCON_DEVCFG_IONU2 (1<<28)
> -#define EP93XX_SYSCON_DEVCFG_GONK (1<<27)
> -#define EP93XX_SYSCON_DEVCFG_TONG (1<<26)
> -#define EP93XX_SYSCON_DEVCFG_MONG (1<<25)
> -#define EP93XX_SYSCON_DEVCFG_U3EN (1<<24)
> -#define EP93XX_SYSCON_DEVCFG_CPENA (1<<23)
> -#define EP93XX_SYSCON_DEVCFG_A2ONG (1<<22)
> -#define EP93XX_SYSCON_DEVCFG_A1ONG (1<<21)
> -#define EP93XX_SYSCON_DEVCFG_U2EN (1<<20)
> -#define EP93XX_SYSCON_DEVCFG_EXVC (1<<19)
> -#define EP93XX_SYSCON_DEVCFG_U1EN (1<<18)
> -#define EP93XX_SYSCON_DEVCFG_TIN (1<<17)
> -#define EP93XX_SYSCON_DEVCFG_HC3IN (1<<15)
> -#define EP93XX_SYSCON_DEVCFG_HC3EN (1<<14)
> -#define EP93XX_SYSCON_DEVCFG_HC1IN (1<<13)
> -#define EP93XX_SYSCON_DEVCFG_HC1EN (1<<12)
> -#define EP93XX_SYSCON_DEVCFG_HONIDE (1<<11)
> -#define EP93XX_SYSCON_DEVCFG_GONIDE (1<<10)
> -#define EP93XX_SYSCON_DEVCFG_PONG (1<<9)
> -#define EP93XX_SYSCON_DEVCFG_EONIDE (1<<8)
> -#define EP93XX_SYSCON_DEVCFG_I2SONSSP (1<<7)
> -#define EP93XX_SYSCON_DEVCFG_I2SONAC97 (1<<6)
> -#define EP93XX_SYSCON_DEVCFG_RASONP3 (1<<4)
> -#define EP93XX_SYSCON_DEVCFG_RAS (1<<3)
> -#define EP93XX_SYSCON_DEVCFG_ADCPD (1<<2)
> -#define EP93XX_SYSCON_DEVCFG_KEYS (1<<1)
> -#define EP93XX_SYSCON_DEVCFG_SHENA (1<<0)
> +#define EP93XX_SYSCON_DEVCFG_SWRST BIT(31)
> +#define EP93XX_SYSCON_DEVCFG_D1ONG BIT(30)
> +#define EP93XX_SYSCON_DEVCFG_D0ONG BIT(29)
> +#define EP93XX_SYSCON_DEVCFG_IONU2 BIT(28)
> +#define EP93XX_SYSCON_DEVCFG_GONK BIT(27)
> +#define EP93XX_SYSCON_DEVCFG_TONG BIT(26)
> +#define EP93XX_SYSCON_DEVCFG_MONG BIT(25)
> +#define EP93XX_SYSCON_DEVCFG_U3EN BIT(24)
> +#define EP93XX_SYSCON_DEVCFG_CPENA BIT(23)
> +#define EP93XX_SYSCON_DEVCFG_A2ONG BIT(22)
> +#define EP93XX_SYSCON_DEVCFG_A1ONG BIT(21)
> +#define EP93XX_SYSCON_DEVCFG_U2EN BIT(20)
> +#define EP93XX_SYSCON_DEVCFG_EXVC BIT(19)
> +#define EP93XX_SYSCON_DEVCFG_U1EN BIT(18)
> +#define EP93XX_SYSCON_DEVCFG_TIN BIT(17)
> +#define EP93XX_SYSCON_DEVCFG_HC3IN BIT(15)
> +#define EP93XX_SYSCON_DEVCFG_HC3EN BIT(14)
> +#define EP93XX_SYSCON_DEVCFG_HC1IN BIT(13)
> +#define EP93XX_SYSCON_DEVCFG_HC1EN BIT(12)
> +#define EP93XX_SYSCON_DEVCFG_HONIDE BIT(11)
> +#define EP93XX_SYSCON_DEVCFG_GONIDE BIT(10)
> +#define EP93XX_SYSCON_DEVCFG_PONG BIT(9)
> +#define EP93XX_SYSCON_DEVCFG_EONIDE BIT(8)
> +#define EP93XX_SYSCON_DEVCFG_I2SONSSP BIT(7)
> +#define EP93XX_SYSCON_DEVCFG_I2SONAC97 BIT(6)
> +#define EP93XX_SYSCON_DEVCFG_RASONP3 BIT(4)
> +#define EP93XX_SYSCON_DEVCFG_RAS BIT(3)
> +#define EP93XX_SYSCON_DEVCFG_ADCPD BIT(2)
> +#define EP93XX_SYSCON_DEVCFG_KEYS BIT(1)
> +#define EP93XX_SYSCON_DEVCFG_SHENA BIT(0)
> #define EP93XX_SYSCON_VIDCLKDIV EP93XX_SYSCON_REG(0x84)
> -#define EP93XX_SYSCON_CLKDIV_ENABLE (1<<15)
> -#define EP93XX_SYSCON_CLKDIV_ESEL (1<<14)
> -#define EP93XX_SYSCON_CLKDIV_PSEL (1<<13)
> +#define EP93XX_SYSCON_CLKDIV_ENABLE BIT(15)
> +#define EP93XX_SYSCON_CLKDIV_ESEL BIT(14)
> +#define EP93XX_SYSCON_CLKDIV_PSEL BIT(13)
> #define EP93XX_SYSCON_CLKDIV_PDIV_SHIFT 8
> #define EP93XX_SYSCON_I2SCLKDIV EP93XX_SYSCON_REG(0x8c)
> -#define EP93XX_SYSCON_I2SCLKDIV_SENA (1<<31)
> -#define EP93XX_SYSCON_I2SCLKDIV_ORIDE (1<<29)
> -#define EP93XX_SYSCON_I2SCLKDIV_SPOL (1<<19)
> +#define EP93XX_SYSCON_I2SCLKDIV_SENA BIT(31)
> +#define EP93XX_SYSCON_I2SCLKDIV_ORIDE BIT(29)
> +#define EP93XX_SYSCON_I2SCLKDIV_SPOL BIT(19)
> #define EP93XX_I2SCLKDIV_SDIV (1 << 16)
> #define EP93XX_I2SCLKDIV_LRDIV32 (0 << 17)
> #define EP93XX_I2SCLKDIV_LRDIV64 (1 << 17)
> #define EP93XX_I2SCLKDIV_LRDIV128 (2 << 17)
> #define EP93XX_I2SCLKDIV_LRDIV_MASK (3 << 17)
> #define EP93XX_SYSCON_KEYTCHCLKDIV EP93XX_SYSCON_REG(0x90)
> -#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN (1<<31)
> -#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV (1<<16)
> -#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN (1<<15)
> -#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV (1<<0)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_TSEN BIT(31)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_ADIV BIT(16)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KEN BIT(15)
> +#define EP93XX_SYSCON_KEYTCHCLKDIV_KDIV BIT(0)
> #define EP93XX_SYSCON_SYSCFG EP93XX_SYSCON_REG(0x9c)
> #define EP93XX_SYSCON_SYSCFG_REV_MASK (0xf0000000)
> #define EP93XX_SYSCON_SYSCFG_REV_SHIFT (28)
> -#define EP93XX_SYSCON_SYSCFG_SBOOT (1<<8)
> -#define EP93XX_SYSCON_SYSCFG_LCSN7 (1<<7)
> -#define EP93XX_SYSCON_SYSCFG_LCSN6 (1<<6)
> -#define EP93XX_SYSCON_SYSCFG_LASDO (1<<5)
> -#define EP93XX_SYSCON_SYSCFG_LEEDA (1<<4)
> -#define EP93XX_SYSCON_SYSCFG_LEECLK (1<<3)
> -#define EP93XX_SYSCON_SYSCFG_LCSN2 (1<<1)
> -#define EP93XX_SYSCON_SYSCFG_LCSN1 (1<<0)
> +#define EP93XX_SYSCON_SYSCFG_SBOOT BIT(8)
> +#define EP93XX_SYSCON_SYSCFG_LCSN7 BIT(7)
> +#define EP93XX_SYSCON_SYSCFG_LCSN6 BIT(6)
> +#define EP93XX_SYSCON_SYSCFG_LASDO BIT(5)
> +#define EP93XX_SYSCON_SYSCFG_LEEDA BIT(4)
> +#define EP93XX_SYSCON_SYSCFG_LEECLK BIT(3)
> +#define EP93XX_SYSCON_SYSCFG_LCSN2 BIT(1)
> +#define EP93XX_SYSCON_SYSCFG_LCSN1 BIT(0)
> #define EP93XX_SYSCON_SWLOCK EP93XX_SYSCON_REG(0xc0)
>
> /* EP93xx System Controller software locked register write */
^ permalink raw reply
* Re: [PATCH net-next v2] hinic: implement the statistical interface of ethtool
From: Jakub Kicinski @ 2019-06-24 19:05 UTC (permalink / raw)
To: Xue Chaojing, davem
Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, chiqijun,
wulike1, Stephen Hemminger
In-Reply-To: <20190624035012.7221-1-xuechaojing@huawei.com>
On Mon, 24 Jun 2019 03:50:12 +0000, Xue Chaojing wrote:
> diff --git a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> index be28a9a7f033..8d98f37c88a8 100644
> --- a/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> +++ b/drivers/net/ethernet/huawei/hinic/hinic_ethtool.c
> @@ -438,6 +438,344 @@ static u32 hinic_get_rxfh_indir_size(struct net_device *netdev)
> return HINIC_RSS_INDIR_SIZE;
> }
>
> +#define ARRAY_LEN(arr) ((int)((int)sizeof(arr) / (int)sizeof(arr[0])))
> +
> +#define HINIC_NETDEV_STAT(_stat_item) { \
> + .name = #_stat_item, \
> + .size = FIELD_SIZEOF(struct rtnl_link_stats64, _stat_item), \
> + .offset = offsetof(struct rtnl_link_stats64, _stat_item) \
> +}
> +
> +static struct hinic_stats hinic_netdev_stats[] = {
> + HINIC_NETDEV_STAT(rx_packets),
> + HINIC_NETDEV_STAT(tx_packets),
> + HINIC_NETDEV_STAT(rx_bytes),
> + HINIC_NETDEV_STAT(tx_bytes),
> + HINIC_NETDEV_STAT(rx_errors),
> + HINIC_NETDEV_STAT(tx_errors),
> + HINIC_NETDEV_STAT(rx_dropped),
> + HINIC_NETDEV_STAT(tx_dropped),
> + HINIC_NETDEV_STAT(multicast),
> + HINIC_NETDEV_STAT(collisions),
> + HINIC_NETDEV_STAT(rx_length_errors),
> + HINIC_NETDEV_STAT(rx_over_errors),
> + HINIC_NETDEV_STAT(rx_crc_errors),
> + HINIC_NETDEV_STAT(rx_frame_errors),
> + HINIC_NETDEV_STAT(rx_fifo_errors),
> + HINIC_NETDEV_STAT(rx_missed_errors),
> + HINIC_NETDEV_STAT(tx_aborted_errors),
> + HINIC_NETDEV_STAT(tx_carrier_errors),
> + HINIC_NETDEV_STAT(tx_fifo_errors),
> + HINIC_NETDEV_STAT(tx_heartbeat_errors),
> +};
I think we wanted to stop duplicating standard netdev stats in ethtool
-S. Chaojing please post a patch to remove this part, the other stats
are good.
^ permalink raw reply
* Re: [PATCH] perf cs-etm: Improve completeness for kernel address space
From: Arnaldo Carvalho de Melo @ 2019-06-24 19:00 UTC (permalink / raw)
To: Leo Yan
Cc: Mathieu Poirier, Linux Kernel Mailing List, linux-arm-kernel,
netdev, bpf, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
Peter Zijlstra, Suzuki Poulouse, Coresight ML
In-Reply-To: <20190620005829.GH24549@leoy-ThinkPad-X240s>
Em Thu, Jun 20, 2019 at 08:58:29AM +0800, Leo Yan escreveu:
> Hi Mathieu,
>
> On Wed, Jun 19, 2019 at 11:49:44AM -0600, Mathieu Poirier wrote:
>
> [...]
>
> > > diff --git a/tools/perf/Makefile.config b/tools/perf/Makefile.config
> > > index 51dd00f65709..4776c2c1fb6d 100644
> > > --- a/tools/perf/Makefile.config
> > > +++ b/tools/perf/Makefile.config
> > > @@ -418,6 +418,30 @@ ifdef CORESIGHT
> > > endif
> > > LDFLAGS += $(LIBOPENCSD_LDFLAGS)
> > > EXTLIBS += $(OPENCSDLIBS)
> > > + ifneq ($(wildcard $(srctree)/arch/arm64/kernel/vmlinux.lds),)
> > > + # Extract info from lds:
> > > + # . = ((((((((0xffffffffffffffff)) - (((1)) << (48)) + 1) + (0)) + (0x08000000))) + (0x08000000))) + 0x00080000;
> > > + # ARM64_PRE_START_SIZE := (0x08000000 + 0x08000000 + 0x00080000)
> > > + ARM64_PRE_START_SIZE := $(shell egrep ' \. \= \({8}0x[0-9a-fA-F]+\){2}' \
> > > + $(srctree)/arch/arm64/kernel/vmlinux.lds | \
> > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > + awk -F' ' '{print "("$$6 "+" $$7 "+" $$8")"}' 2>/dev/null)
> > > + else
> > > + ARM64_PRE_START_SIZE := 0
> > > + endif
> > > + CFLAGS += -DARM64_PRE_START_SIZE="$(ARM64_PRE_START_SIZE)"
> > > + ifneq ($(wildcard $(srctree)/arch/arm/kernel/vmlinux.lds),)
> > > + # Extract info from lds:
> > > + # . = ((0xC0000000)) + 0x00208000;
> > > + # ARM_PRE_START_SIZE := 0x00208000
> > > + ARM_PRE_START_SIZE := $(shell egrep ' \. \= \({2}0x[0-9a-fA-F]+\){2}' \
> > > + $(srctree)/arch/arm/kernel/vmlinux.lds | \
> > > + sed -e 's/[(|)|.|=|+|<|;|-]//g' -e 's/ \+/ /g' -e 's/^[ \t]*//' | \
> > > + awk -F' ' '{print "("$$2")"}' 2>/dev/null)
> > > + else
> > > + ARM_PRE_START_SIZE := 0
> > > + endif
> > > + CFLAGS += -DARM_PRE_START_SIZE="$(ARM_PRE_START_SIZE)"
> > > $(call detected,CONFIG_LIBOPENCSD)
> > > ifdef CSTRACE_RAW
> > > CFLAGS += -DCS_DEBUG_RAW
> > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
> > > index 0c7776b51045..ae831f836c70 100644
> > > --- a/tools/perf/util/cs-etm.c
> > > +++ b/tools/perf/util/cs-etm.c
> > > @@ -613,10 +613,34 @@ static void cs_etm__free(struct perf_session *session)
> > > static u8 cs_etm__cpu_mode(struct cs_etm_queue *etmq, u64 address)
> > > {
> > > struct machine *machine;
> > > + u64 fixup_kernel_start = 0;
> > > + const char *arch;
> > >
> > > machine = etmq->etm->machine;
> > > + arch = perf_env__arch(machine->env);
> > >
> > > - if (address >= etmq->etm->kernel_start) {
> > > + /*
> > > + * Since arm and arm64 specify some memory regions prior to
> > > + * 'kernel_start', kernel addresses can be less than 'kernel_start'.
> > > + *
> > > + * For arm architecture, the 16MB virtual memory space prior to
> > > + * 'kernel_start' is allocated to device modules, a PMD table if
> > > + * CONFIG_HIGHMEM is enabled and a PGD table.
> > > + *
> > > + * For arm64 architecture, the root PGD table, device module memory
> > > + * region and BPF jit region are prior to 'kernel_start'.
> > > + *
> > > + * To reflect the complete kernel address space, compensate these
> > > + * pre-defined regions for kernel start address.
> > > + */
> > > + if (!strcmp(arch, "arm64"))
> > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > + ARM64_PRE_START_SIZE;
> > > + else if (!strcmp(arch, "arm"))
> > > + fixup_kernel_start = etmq->etm->kernel_start -
> > > + ARM_PRE_START_SIZE;
> >
> > I will test your work but from a quick look wouldn't it be better to
> > have a single define name here? From looking at the modifications you
> > did to Makefile.config there doesn't seem to be a reason to have two.
>
> Thanks for suggestion. I changed to use single define
> ARM_PRE_START_SIZE and sent patch v2 [1].
>
> If possible, please test patch v2.
>
> Thanks,
> Leo Yan
So just for the record, I'm waiting for Mathieu on this one, i.e. for
him to test/ack v3.
- Arnaldo
> [1] https://lore.kernel.org/linux-arm-kernel/20190620005428.20883-1-leo.yan@linaro.org/T/#u
>
> > > +
> > > + if (address >= fixup_kernel_start) {
> > > if (machine__is_host(machine))
> > > return PERF_RECORD_MISC_KERNEL;
> > > else
> > > --
> > > 2.17.1
> > >
--
- Arnaldo
^ permalink raw reply
* Re: selftests: bpf: test_libbpf.sh failed at file test_l4lb.o
From: Andrii Nakryiko @ 2019-06-24 18:32 UTC (permalink / raw)
To: Dan Rue
Cc: Naresh Kamboju, open list:KERNEL SELFTEST FRAMEWORK, bpf, Netdev,
open list, xdp-newbies, David S. Miller, Daniel Borkmann,
Alexei Starovoitov, Martin Lau, Yonghong Song, john fastabend,
hawk, Jakub Kicinski, Shuah Khan
In-Reply-To: <20190621161752.d7d7n4m5q67uivys@xps.therub.org>
On Fri, Jun 21, 2019 at 9:17 AM Dan Rue <dan.rue@linaro.org> wrote:
>
> On Thu, Jun 20, 2019 at 10:17:04PM -0700, Andrii Nakryiko wrote:
> > On Thu, Jun 20, 2019 at 1:08 AM Naresh Kamboju
> > <naresh.kamboju@linaro.org> wrote:
> > >
> > > selftests: bpf test_libbpf.sh failed running Linux -next kernel
> > > 20190618 and 20190619.
> > >
> > > Here is the log from x86_64,
> > > # selftests bpf test_libbpf.sh
> > > bpf: test_libbpf.sh_ #
> > > # [0] libbpf BTF is required, but is missing or corrupted.
> >
> > You need at least clang-9.0.0 (not yet released) to run some of these
> > tests successfully, as they rely on Clang's support for
> > BTF_KIND_VAR/BTF_KIND_DATASEC.
>
> Can there be a runtime check for BTF that emits a skip instead of a fail
> in such a case?
I'm not sure how to do this simply and minimally intrusively. The best
I can come up with is setting some envvar from Makefile and checking
for that in each inidividual test, which honestly sounds a bit gross.
How hard is it for you guys to upgrade compiler used to run these test?
>
> Thanks,
> Dan
>
> >
> > > libbpf: BTF_is #
> > > # test_libbpf failed at file test_l4lb.o
> > > failed: at_file #
> > > # selftests test_libbpf [FAILED]
> > > test_libbpf: [FAILED]_ #
> > > [FAIL] 29 selftests bpf test_libbpf.sh
> > > selftests: bpf_test_libbpf.sh [FAIL]
> > >
> > > Full test log,
> > > https://qa-reports.linaro.org/lkft/linux-next-oe/build/next-20190619/testrun/781777/log
> > >
> > > Test results comparison,
> > > https://qa-reports.linaro.org/lkft/linux-next-oe/tests/kselftest/bpf_test_libbpf.sh
> > >
> > > Good linux -next tag: next-20190617
> > > Bad linux -next tag: next-20190618
> > > git branch master
> > > git commit 1c6b40509daf5190b1fd2c758649f7df1da4827b
> > > git repo
> > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
> > >
> > > Best regards
> > > Naresh Kamboju
>
> --
> Linaro - Kernel Validation
^ permalink raw reply
* Re: [PATCH net-next 4/6] arm64: dts: fsl: ls1028a: Add Felix switch port DT node
From: Alexandre Belloni @ 2019-06-24 18:26 UTC (permalink / raw)
To: Andrew Lunn
Cc: Allan W. Nielsen, Claudiu Manoil, David S . Miller,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
UNGLinuxDriver@microchip.com, Allan Nielsen, Rob Herring,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190624162431.GX31306@lunn.ch>
On 24/06/2019 18:24:31+0200, Andrew Lunn wrote:
> On Mon, Jun 24, 2019 at 05:23:45PM +0200, Allan W. Nielsen wrote:
> > Hi Andrew,
> >
> > The 06/24/2019 16:26, Andrew Lunn wrote:
> > > > > Yeah, there are 2 ethernet controller ports (managed by the enetc driver)
> > > > > connected inside the SoC via SGMII links to 2 of the switch ports, one of
> > > > > these switch ports can be configured as CPU port (with follow-up patches).
> > > > >
> > > > > This configuration may look prettier on DSA, but the main restriction here
> > > > > is that the entire functionality is provided by the ocelot driver which is a
> > > > > switchdev driver. I don't think it would be a good idea to copy-paste code
> > > > > from ocelot to a separate dsa driver.
> > > > >
> > > >
> > > > We should probably make the ocelot driver a DSA driver then...
> > > An important part of DSA is being able to direct frames out specific
> > > ports when they ingress via the CPU port. Does the silicon support
> > > this? At the moment, i think it is using polled IO.
> >
> > That is supported, it requires a bit of initial configuration of the Chip, but
> > nothing big (I believe this configuration is part of Claudiu's change-set).
> >
> > But how do you envision this done?
> >
> > - Let the existing SwitchDev driver and the DSA driver use a set of common
> > functions.
> > - Convert the existing Ocelot driver from SwitchDev to DSA
> > - Fork (copy) the existing driver of Ocelot, and modify it as needed for the
> > Felix driver
> >
> > My guess is the first one, but I would like to understand what you have in mind.
>
> I don't know the various architectures the switch is used in. But it
> does seem like a core library, and then a switchdev wrapper for Ocelot
> and a DSA wrapper for Felix would make sense.
Ocelot could also be used in a DSA setting where one port can be
connected to an external MAC and be used to inject/extract frames
to/from any other ports. In that case, the IFH would serve as the DSA
tag.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v2 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: kbuild test robot @ 2019-06-24 18:24 UTC (permalink / raw)
To: Raju Rangoju; +Cc: kbuild-all, netdev, davem, nirranjan, dt, rajur
In-Reply-To: <20190624085037.2358-5-rajur@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 2908 bytes --]
Hi Raju,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Raju-Rangoju/cxgb4-Reference-count-MPS-TCAM-entries-within-a-PF/20190624-230630
config: x86_64-randconfig-b0-06242344 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c: In function 'cxgb4_mps_ref_dec_by_mac':
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:29: warning: passing argument 1 of 'atomic_dec_and_test' from incompatible pointer type
if (!atomic_dec_and_test(&mps_entry->refcnt)) {
^
In file included from arch/x86/include/asm/atomic.h:265:0,
from arch/x86/include/asm/msr.h:67,
from arch/x86/include/asm/processor.h:21,
from arch/x86/include/asm/cpufeature.h:5,
from arch/x86/include/asm/thread_info.h:53,
from include/linux/thread_info.h:38,
from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/hardirq.h:5,
from include/linux/interrupt.h:11,
from drivers/net/ethernet/chelsio/cxgb4/cxgb4.h:42,
from drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:4:
include/asm-generic/atomic-instrumented.h:745:1: note: expected 'struct atomic_t *' but argument is of type 'struct refcount_t *'
atomic_dec_and_test(atomic_t *v)
^
vim +/atomic_dec_and_test +17 drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
5
6 static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
7 const u8 *addr, const u8 *mask)
8 {
9 u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
10 struct mps_entries_ref *mps_entry, *tmp;
11 int ret = -EINVAL;
12
13 spin_lock_bh(&adap->mps_ref_lock);
14 list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
15 if (ether_addr_equal(mps_entry->addr, addr) &&
16 ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
> 17 if (!atomic_dec_and_test(&mps_entry->refcnt)) {
18 spin_unlock_bh(&adap->mps_ref_lock);
19 return -EBUSY;
20 }
21 list_del(&mps_entry->list);
22 kfree(mps_entry);
23 ret = 0;
24 break;
25 }
26 }
27 spin_unlock_bh(&adap->mps_ref_lock);
28 return ret;
29 }
30
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35462 bytes --]
^ permalink raw reply
* Re: [PATCH] samples: bpf: make the use of xdp samples consistent
From: Toke Høiland-Jørgensen @ 2019-06-24 18:24 UTC (permalink / raw)
To: Daniel T. Lee, Daniel Borkmann, Alexei Starovoitov; +Cc: netdev
In-Reply-To: <20190624112009.20048-1-danieltimlee@gmail.com>
"Daniel T. Lee" <danieltimlee@gmail.com> writes:
> Currently, each xdp samples are inconsistent in the use.
> Most of the samples fetch the interface with it's name.
> (ex. xdp1, xdp2skb, xdp_redirect, xdp_sample_pkts, etc.)
The xdp_redirect and xdp_redirect_map also only accept ifindexes, not
interface names. Care to fix those while you're at it? :)
-Toke
^ permalink raw reply
* Re: [PATCH iproute2] iproute: Pass RTM_F_CLONED on dump to fetch cached routes to be flushed
From: Stefano Brivio @ 2019-06-24 18:20 UTC (permalink / raw)
To: Stephen Hemminger, David Ahern
Cc: David Miller, Jianlin Shi, Wei Wang, Martin KaFai Lau,
Eric Dumazet, Matti Vaittinen, netdev
In-Reply-To: <7ae318a8b632c216df95362524cd4bb5f4f1f537.1560561439.git.sbrivio@redhat.com>
Stephen,
On Sat, 15 Jun 2019 03:33:50 +0200
Stefano Brivio <sbrivio@redhat.com> wrote:
> With a current (5.1) kernel version, IPv6 exception routes can't be listed
> (ip -6 route list cache) or flushed (ip -6 route flush cache). I'm
> re-introducing kernel support for this, but, to allow the kernel to filter
> routes based on the RTM_F_CLONED flag, we need to make sure this flag is
> always passed when we want cached routes to be dumped.
Support for listing IPv6 route exceptions is now back on net-next,
relevant commits:
564c91f7e563 fib_frontend, ip6_fib: Select routes or exceptions dump from RTM_F_CLONED
ef11209d4219 Revert "net/ipv6: Bail early if user only wants cloned entries"
3401bfb1638e ipv6/route: Don't match on fc_nh_id if not set in ip6_route_del()
bf9a8a061ddc ipv6/route: Change return code of rt6_dump_route() for partial node dumps
1e47b4837f3b ipv6: Dump route exceptions if requested
40cb35d5dc04 ip6_fib: Don't discard nodes with valid routing information in fib6_locate_1()
and this iproute2 patch works together with that as it is. Do I need to
re-submit? Thanks.
--
Stefano
^ permalink raw reply
* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nicolas Dichtel @ 2019-06-24 18:18 UTC (permalink / raw)
To: Nick Desaulniers, David Miller; +Cc: netdev, kbuild test robot
In-Reply-To: <CAKwvOdkqE_RVosXAe9ULePR8A37CHh6+JtDMaRAghUA41Y_+yg@mail.gmail.com>
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.
Regards,
Nicolas
^ permalink raw reply
* Re: [PATCH v3 net] af_packet: Block execution of tasks waiting for transmit to complete in AF_PACKET
From: Willem de Bruijn @ 2019-06-24 18:08 UTC (permalink / raw)
To: Neil Horman; +Cc: Network Development, Matteo Croce, David S. Miller
In-Reply-To: <20190624004604.25607-1-nhorman@tuxdriver.com>
On Sun, Jun 23, 2019 at 8:46 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> When an application is run that:
> a) Sets its scheduler to be SCHED_FIFO
> and
> b) Opens a memory mapped AF_PACKET socket, and sends frames with the
> MSG_DONTWAIT flag cleared, its possible for the application to hang
> forever in the kernel. This occurs because when waiting, the code in
> tpacket_snd calls schedule, which under normal circumstances allows
> other tasks to run, including ksoftirqd, which in some cases is
> responsible for freeing the transmitted skb (which in AF_PACKET calls a
> destructor that flips the status bit of the transmitted frame back to
> available, allowing the transmitting task to complete).
>
> However, when the calling application is SCHED_FIFO, its priority is
> such that the schedule call immediately places the task back on the cpu,
> preventing ksoftirqd from freeing the skb, which in turn prevents the
> transmitting task from detecting that the transmission is complete.
>
> We can fix this by converting the schedule call to a completion
> mechanism. By using a completion queue, we force the calling task, when
> it detects there are no more frames to send, to schedule itself off the
> cpu until such time as the last transmitted skb is freed, allowing
> forward progress to be made.
>
> Tested by myself and the reporter, with good results
>
> Appies to the net tree
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> Reported-by: Matteo Croce <mcroce@redhat.com>
> CC: "David S. Miller" <davem@davemloft.net>
> CC: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
>
> Change Notes:
>
> V1->V2:
> Enhance the sleep logic to support being interruptible and
> allowing for honoring to SK_SNDTIMEO (Willem de Bruijn)
>
> V2->V3:
> Rearrage the point at which we wait for the completion queue, to
> avoid needing to check for ph/skb being null at the end of the loop.
> Also move the complete call to the skb destructor to avoid needing to
> modify __packet_set_status. Also gate calling complete on
> packet_read_pending returning zero to avoid multiple calls to complete.
> (Willem de Bruijn)
>
> Move timeo computation within loop, to re-fetch the socket
> timeout since we also use the timeo variable to record the return code
> from the wait_for_complete call (Neil Horman)
> ---
> net/packet/af_packet.c | 59 +++++++++++++++++++++++++++++++++++++-----
> net/packet/internal.h | 2 ++
> 2 files changed, 55 insertions(+), 6 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index a29d66da7394..5c48bb7a4fa5 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -380,7 +380,6 @@ static void __packet_set_status(struct packet_sock *po, void *frame, int status)
> WARN(1, "TPACKET version not supported.\n");
> BUG();
> }
> -
Unrelated to this feature
> smp_wmb();
> }
>
> @@ -1148,6 +1147,14 @@ static void *packet_previous_frame(struct packet_sock *po,
> return packet_lookup_frame(po, rb, previous, status);
> }
>
> +static void *packet_next_frame(struct packet_sock *po,
> + struct packet_ring_buffer *rb,
> + int status)
> +{
> + unsigned int next = rb->head != rb->frame_max ? rb->head+1 : 0;
> + return packet_lookup_frame(po, rb, next, status);
> +}
> +
> static void packet_increment_head(struct packet_ring_buffer *buff)
> {
> buff->head = buff->head != buff->frame_max ? buff->head+1 : 0;
> @@ -2401,6 +2408,9 @@ static void tpacket_destruct_skb(struct sk_buff *skb)
>
> ts = __packet_set_timestamp(po, ph, skb);
> __packet_set_status(po, ph, TP_STATUS_AVAILABLE | ts);
> +
> + if (po->wait_on_complete && !packet_read_pending(&po->tx_ring))
> + complete(&po->skb_completion);
> }
>
> sock_wfree(skb);
> @@ -2600,9 +2610,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> int len_sum = 0;
> int status = TP_STATUS_AVAILABLE;
> int hlen, tlen, copylen = 0;
> + long timeo = 0;
>
> mutex_lock(&po->pg_vec_lock);
>
> +
Same
> if (likely(saddr == NULL)) {
> dev = packet_cached_dev_get(po);
> proto = po->num;
> @@ -2647,16 +2659,16 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> size_max = dev->mtu + reserve + VLAN_HLEN;
>
> do {
> +
Same
> ph = packet_current_frame(po, &po->tx_ring,
> TP_STATUS_SEND_REQUEST);
> - if (unlikely(ph == NULL)) {
> - if (need_wait && need_resched())
> - schedule();
> - continue;
Why not keep the test whether the process needs to wait exactly here (A)?
Then no need for packet_next_frame.
> - }
> +
> + if (unlikely(ph == NULL))
> + break;
>
> skb = NULL;
> tp_len = tpacket_parse_header(po, ph, size_max, &data);
> +
Again
> if (tp_len < 0)
> goto tpacket_error;
>
> @@ -2720,6 +2732,21 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
>
> skb->destructor = tpacket_destruct_skb;
> __packet_set_status(po, ph, TP_STATUS_SENDING);
> +
> + /*
> + * If we need to wait and we've sent the last frame pending
> + * transmission in the mmaped buffer, flag that we need to wait
> + * on those frames to get freed via tpacket_destruct_skb. This
> + * flag indicates that tpacket_destruct_skb should call complete
> + * when the packet_pending count reaches zero, and that we need
> + * to call wait_on_complete_interruptible_timeout below, to make
> + * sure we pick up the result of that completion
> + */
> + if (need_wait && !packet_next_frame(po, &po->tx_ring, TP_STATUS_SEND_REQUEST)) {
> + po->wait_on_complete = 1;
> + timeo = sock_sndtimeo(&po->sk, msg->msg_flags & MSG_DONTWAIT);
This resets timeout on every loop. should only set above the loop once.
Also, please limit the comments in the code (also below). If every
patch would add this many lines of comment, the file would be
enormous. OTOH, it's great to be this explanatory in the git commit,
which is easily reached for any line with git blame.
> + }
> +
> packet_inc_pending(&po->tx_ring);
>
> status = TP_STATUS_SEND_REQUEST;
> @@ -2728,6 +2755,11 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> err = net_xmit_errno(err);
> if (err && __packet_get_status(po, ph) ==
> TP_STATUS_AVAILABLE) {
> + /* re-init completion queue to avoid subsequent fallthrough
> + * on a future thread calling wait_on_complete_interruptible_timeout
> + */
> + po->wait_on_complete = 0;
If setting where sleeping, no need for resetting if a failure happens
between those blocks.
> + init_completion(&po->skb_completion);
no need to reinit between each use?
> /* skb was destructed already */
> skb = NULL;
> goto out_status;
> @@ -2740,6 +2772,20 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
> }
> packet_increment_head(&po->tx_ring);
> len_sum += tp_len;
> +
> + if (po->wait_on_complete) {
> + timeo = wait_for_completion_interruptible_timeout(&po->skb_completion, timeo);
> + po->wait_on_complete = 0;
I was going to argue for clearing in tpacket_destruct_skb. But then we
would have to separate clear on timeout instead of signal, too.
po->wait_on_complete = 1;
timeo = wait_for_completion...
po->wait_on_complete = 0;
as the previous version had is fine, as long as the compiler does not
"optimize" away an assignment. The function call will avoid reordering
by the cpu, at least. Probably requires WRITE_ONCE/READ_ONCE.
> + if (!timeo) {
> + /* We timed out, break out and notify userspace */
> + err = -ETIMEDOUT;
> + goto out_status;
goto out_put, there is no active ph or skb here
> + } else if (timeo == -ERESTARTSYS) {
> + err = -ERESTARTSYS;
> + goto out_status;
> + }
> + }
> +
> } while (likely((ph != NULL) ||
> /* Note: packet_read_pending() might be slow if we have
> * to call it as it's per_cpu variable, but in fast-path
> @@ -3207,6 +3253,7 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> sock_init_data(sock, sk);
>
> po = pkt_sk(sk);
> + init_completion(&po->skb_completion);
> sk->sk_family = PF_PACKET;
> po->num = proto;
> po->xmit = dev_queue_xmit;
This is basically replacing a busy-wait with schedule() with sleeping
using wait_for_completion_interruptible_timeout. My main question is
does this really need to move control flow around and add
packet_next_frame? If not, especially for net, the shortest, simplest
change is preferable.
^ permalink raw reply
* Re: [PATCH rdma-next v1 12/12] IB/mlx5: Add DEVX support for CQ events
From: Jason Gunthorpe @ 2019-06-24 18:06 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <a076a050-871b-c468-f62e-95bb4f0ac2c2@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 08:03:07PM +0300, Yishai Hadas wrote:
> On 6/24/2019 3:04 PM, Jason Gunthorpe wrote:
> > On Tue, Jun 18, 2019 at 08:15:40PM +0300, Leon Romanovsky wrote:
> > > From: Yishai Hadas <yishaih@mellanox.com>
> > >
> > > Add DEVX support for CQ events by creating and destroying the CQ via
> > > mlx5_core and set an handler to manage its completions.
> > >
> > > Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > drivers/infiniband/hw/mlx5/devx.c | 40 +++++++++++++++++++++++++++++++
> > > 1 file changed, 40 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
> > > index 49fdce95d6d9..91ccd58ebc05 100644
> > > +++ b/drivers/infiniband/hw/mlx5/devx.c
> > > @@ -19,9 +19,12 @@
> > > #define UVERBS_MODULE_NAME mlx5_ib
> > > #include <rdma/uverbs_named_ioctl.h>
> > > +static void dispatch_event_fd(struct list_head *fd_list, const void *data);
> > > +
> > > enum devx_obj_flags {
> > > DEVX_OBJ_FLAGS_INDIRECT_MKEY = 1 << 0,
> > > DEVX_OBJ_FLAGS_DCT = 1 << 1,
> > > + DEVX_OBJ_FLAGS_CQ = 1 << 2,
> > > };
> > > struct devx_async_data {
> > > @@ -94,6 +97,7 @@ struct devx_async_event_file {
> > > #define MLX5_MAX_DESTROY_INBOX_SIZE_DW MLX5_ST_SZ_DW(delete_fte_in)
> > > struct devx_obj {
> > > struct mlx5_core_dev *mdev;
> > > + struct mlx5_ib_dev *ib_dev;
> >
> > This seems strange, why would we need to store the core_dev and the ib_dev
> > in a struct when ibdev->mdev == core_dev?
> >
>
> We need to add the ib_dev as we can't access it from the core_dev.
> Most of this patch we can probably go and drop the mdev and access it from
> ib_dev, I preferred to not handle that in this patch.
Should add a patch to revise it then
Jason
^ permalink raw reply
* Re: [PATCH rdma-next v1 11/12] IB/mlx5: Implement DEVX dispatching event
From: Jason Gunthorpe @ 2019-06-24 18:06 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <3a2e53f8-e7dd-3e01-c7c7-99d41f711d87@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 07:55:32PM +0300, Yishai Hadas wrote:
> > > + /* Explicit filtering to kernel events which may occur frequently */
> > > + if (event_type == MLX5_EVENT_TYPE_CMD ||
> > > + event_type == MLX5_EVENT_TYPE_PAGE_REQUEST)
> > > + return NOTIFY_OK;
> > > +
> > > + table = container_of(nb, struct mlx5_devx_event_table, devx_nb.nb);
> > > + dev = container_of(table, struct mlx5_ib_dev, devx_event_table);
> > > + is_unaffiliated = is_unaffiliated_event(dev->mdev, event_type);
> > > +
> > > + if (!is_unaffiliated)
> > > + obj_type = get_event_obj_type(event_type, data);
> > > + event = xa_load(&table->event_xa, event_type | (obj_type << 16));
> > > + if (!event)
> > > + return NOTIFY_DONE;
> >
> > event should be in the rcu as well
>
> Do we really need this ? I didn't see a flow that really requires
> that.
I think there are no frees left? Even so it makes much more sense to
include the event in the rcu as if we ever did need to kfree it would
have to be via rcu
> > > + while (list_empty(&ev_queue->event_list)) {
> > > + spin_unlock_irq(&ev_queue->lock);
> > > +
> > > + if (filp->f_flags & O_NONBLOCK)
> > > + return -EAGAIN;
> > > +
> > > + if (wait_event_interruptible(ev_queue->poll_wait,
> > > + (!list_empty(&ev_queue->event_list) ||
> > > + ev_queue->is_destroyed))) {
> > > + return -ERESTARTSYS;
> > > + }
> > > +
> > > + if (list_empty(&ev_queue->event_list) &&
> > > + ev_queue->is_destroyed)
> > > + return -EIO;
> >
> > All these tests should be under the lock.
>
> We can't call wait_event_interruptible() above which may sleep under the
> lock, correct ? are you referring to the list_empty() and
> is_destroyed ?
yes
> By the way looking in uverb code [1], similar code which is not done under
> the lock as of here..
>
> [1] https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L244
Also not a good idea
> > Why don't we return EIO as soon as is-destroyed happens? What is the
> > point of flushing out the accumulated events?
>
> It follows the above uverb code/logic that returns existing events even in
> that case, also the async command events in this file follows that logic, I
> suggest to stay consistent.
Don't follow broken uverbs stuff...
> > Maybe the event should be re-added on error? Tricky.
>
> What will happen if another copy_to_user may then fail again (loop ?) ...
> not sure that we want to get into this tricky handling ...
>
> As of above, It follows the logic from uverbs at that area.
> https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L267
again it is wrong...
There is no loop if you just stick the item back on the head of the
list and exit, which is probably the right thing to do..
> > > @@ -2374,6 +2705,17 @@ static int devx_hot_unplug_async_cmd_event_file(struct ib_uobject *uobj,
> > > static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
> > > enum rdma_remove_reason why)
> > > {
> > > + struct devx_async_event_file *ev_file =
> > > + container_of(uobj, struct devx_async_event_file,
> > > + uobj);
> > > + struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
> > > +
> > > + spin_lock_irq(&ev_queue->lock);
> > > + ev_queue->is_destroyed = 1;
> > > + spin_unlock_irq(&ev_queue->lock);
> > > +
> > > + if (why == RDMA_REMOVE_DRIVER_REMOVE)
> > > + wake_up_interruptible(&ev_queue->poll_wait);
> >
> > Why isn't this wakeup always done?
>
> Maybe you are right and this can be always done to wake up any readers as
> the 'is_destroyed' was set.
>
> By the way, any idea why it was done as such in uverbs [1] for similar flow
> ? also the command events follows that.
I don't know, it is probably pointless too.
If we don't need it here then we shouldn't have it.
These random pointless ifs bother me as we have to spend time trying
to figure out that they are pointless down the road.
Jason
^ permalink raw reply
* Re: [PATCH net-next v2 0/4] net/sched: Introduce tc connection tracking
From: Cong Wang @ 2019-06-24 17:59 UTC (permalink / raw)
To: Paul Blakey
Cc: Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
Marcelo Ricardo Leitner, Linux Kernel Network Developers,
David Miller, Aaron Conole, Zhike Wang, Rony Efraim, nst-kernel,
John Hurley, Simon Horman, Justin Pettit
In-Reply-To: <1561038141-31370-1-git-send-email-paulb@mellanox.com>
On Thu, Jun 20, 2019 at 6:43 AM Paul Blakey <paulb@mellanox.com> wrote:
>
> Hi,
>
> This patch series add connection tracking capabilities in tc sw datapath.
> It does so via a new tc action, called act_ct, and new tc flower classifier matching
> on conntrack state, mark and label.
Thanks for more detailed description here.
I still don't see why we have to do this in L2, mind to be more specific?
IOW, if you really want to manipulate conntrack info and use it for
matching, why not do it in netfilter layer as it is where conntrack is?
BTW, if the cls_flower ct_state matching is not in upstream yet, please
try to push it first, as it is a justification of this patchset.
Thanks.
^ permalink raw reply
* Re: [PATCH iproute2-next v3 1/2] ipaddress: correctly print a VF hw address in the IPoIB case
From: David Ahern @ 2019-06-24 17:59 UTC (permalink / raw)
To: Denis Kirjanov, stephen; +Cc: netdev, linux-rdma, dledford, mkubecek
In-Reply-To: <20190622180035.40245-1-dkirjanov@suse.com>
On 6/22/19 12:00 PM, Denis Kirjanov wrote:
> @@ -365,13 +367,45 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
> parse_rtattr_nested(vf, IFLA_VF_MAX, vfinfo);
>
> vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
> + vf_broadcast = RTA_DATA(vf[IFLA_VF_BROADCAST]);
> vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
>
> print_string(PRINT_FP, NULL, "%s ", _SL_);
> print_int(PRINT_ANY, "vf", "vf %d ", vf_mac->vf);
> - print_string(PRINT_ANY, "mac", "MAC %s",
> - ll_addr_n2a((unsigned char *) &vf_mac->mac,
> - ETH_ALEN, 0, b1, sizeof(b1)));
> +
> + print_string(PRINT_ANY,
> + "link_type",
> + " link/%s ",
> + ll_type_n2a(ifi->ifi_type, b1, sizeof(b1)));
> +
> + print_color_string(PRINT_ANY,
> + COLOR_MAC,
> + "address",
> + "%s",
> + ll_addr_n2a((unsigned char *) &vf_mac->mac,
> + ifi->ifi_type == ARPHRD_ETHER ?
> + ETH_ALEN : INFINIBAND_ALEN,
> + ifi->ifi_type,
> + b1, sizeof(b1)));
you still have a lot of lines that are not lined up column wise. See how
the COLOR_MAC is offset to the right from PRINT_ANY?
> +
> + if (vf[IFLA_VF_BROADCAST]) {
> + if (ifi->ifi_flags&IFF_POINTOPOINT) {
> + print_string(PRINT_FP, NULL, " peer ", NULL);
> + print_bool(PRINT_JSON,
> + "link_pointtopoint", NULL, true);
> + } else
> + print_string(PRINT_FP, NULL, " brd ", NULL);
> +
> + print_color_string(PRINT_ANY,
> + COLOR_MAC,
> + "broadcast",
> + "%s",
> + ll_addr_n2a((unsigned char *) &vf_broadcast->broadcast,
> + ifi->ifi_type == ARPHRD_ETHER ?
> + ETH_ALEN : INFINIBAND_ALEN,
> + ifi->ifi_type,
> + b1, sizeof(b1)));
And then these lines are offset to the left.
^ permalink raw reply
* Re: [PATCH rdma-next v1 10/12] IB/mlx5: Enable subscription for device events over DEVX
From: Jason Gunthorpe @ 2019-06-24 17:56 UTC (permalink / raw)
To: Yishai Hadas
Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <33f9402b-ccae-b874-cc72-b6afb1fb8655@dev.mellanox.co.il>
On Mon, Jun 24, 2019 at 07:13:14PM +0300, Yishai Hadas wrote:
> > > + u32 xa_key_level1;
> > > + u32 xa_key_level2;
> > > + struct rcu_head rcu;
> > > + u64 cookie;
> > > + bool is_obj_related;
> > > + struct ib_uobject *fd_uobj;
> > > + void *object; /* May need direct access upon hot unplug */
> >
> > This should be a 'struct file *' and have a better name.
> >
>
> OK, will change.
>
> > And I'm unclear why we need to store both the ib_uobject and the
> > struct file for the same thing?
>
> Post hot unplug/unbind the uobj can't be accessed any more to reach the
> object as it will be set to NULL by ib_core layer [1].
struct file users need to get the uobject from the file->private_data
under a fget.
There is only place place that needed fd_uobj, and it was under the
fget section, so it should simply use private_data.
This is why you should only store the struct file and not the uobject.
> This was the comment that I have just put above in the code, I may improve
> it with more details as pointed here.
>
> [1]
> https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/rdma_core.c#L149
I'm wondering if this is a bug to do this for fds?
> > Since uobj->object == flip && filp->private_data == uobj, I have a
> > hard time to understand why we need both things, it seems to me that
> > if we get the fget on the filp then we can rely on the
> > filp->private_data to get back to the devx_async_event_file.
> >
>
> The idea was to not take an extra ref count on the file (i.e. fget) per
> subscription, this will let the release option to be called once the file
> will be closed by the application.
No extra ref is needed, the fget is already obtained in the only place
that needs fd_uobj.
> > > + obj_event = xa_load(&event->object_ids, key_level2);
> > > + if (!obj_event) {
> > > + err = xa_reserve(&event->object_ids, key_level2, GFP_KERNEL);
> > > + if (err)
> > > + goto err_level1;
> > > +
> > > + obj_event = kzalloc(sizeof(*obj_event), GFP_KERNEL);
> > > + if (!obj_event) {
> > > + err = -ENOMEM;
> > > + goto err_level2;
> > > + }
> > > +
> > > + INIT_LIST_HEAD(&obj_event->obj_sub_list);
> > > + *alloc_obj_event = obj_event;
> >
> > This is goofy, just store the empty obj_event in the xa instead of
> > using xa_reserve, and when you go to do the error unwind just delete
> > any level2' devx_obj_event' that has a list_empty(obj_sub_list), get
> > rid of the wonky alloc_obj_event stuff.
> >
>
> Please see my answer above about how level2 is managed by this
> alloc_obj_event, is that really worth a change ? I found current logic to be
> clear. I may put some note here if we can stay with that.
I think it is alot cleaner/simpler than using this extra memory
> > The best configuration would be to use devx_cleanup_subscription to
> > undo the partially ready subscription.
>
> This partially ready subscription might not match the
> devx_cleanup_subscription(), e.g. it wasn't added to xa_list and can't be
> deleted without any specific flag to ignore ..
Maybe, but I suspect it can work out
> > > + event_sub_arr = uverbs_zalloc(attrs,
> > > + MAX_NUM_EVENTS * sizeof(struct devx_event_subscription *));
> > > + event_obj_array_alloc = uverbs_zalloc(attrs,
> > > + MAX_NUM_EVENTS * sizeof(struct devx_obj_event *));
> >
> > There are so many list_heads in the devx_event_subscription, why not
> > use just one of them to store the allocated events instead of this
> > temp array? ie event_list looks good for this purpose.
> >
>
> I'm using the array later on with direct access to the index that should be
> de-allocated. I would prefer staying with this array rather than using the
> 'event_list' which has other purpose down the road, it's used per
> subscription and doesn't look match to hold the devx_obj_event which has no
> list entry for this purpose..
Replace the event_obj_array_alloc by storing that data directly in
the xarray
Replace the event_sub_arr by building them into a linked list - it
always need to iterate over the whole list anyhow.
> > > +
> > > + if (!event_sub_arr || !event_obj_array_alloc)
> > > + return -ENOMEM;
> > > +
> > > + /* Protect from concurrent subscriptions to same XA entries to allow
> > > + * both to succeed
> > > + */
> > > + mutex_lock(&devx_event_table->event_xa_lock);
> > > + for (i = 0; i < num_events; i++) {
> > > + u32 key_level1;
> > > +
> > > + if (obj)
> > > + obj_type = get_dec_obj_type(obj,
> > > + event_type_num_list[i]);
> > > + key_level1 = event_type_num_list[i] | obj_type << 16;
> > > +
> > > + err = subscribe_event_xa_alloc(devx_event_table,
> > > + key_level1,
> > > + obj ? true : false,
> > > + obj_id,
> > > + &event_obj_array_alloc[i]);
> >
> > Usless ?:
>
> What do you suggest instead ?
Nothing is needed, cast to implicit bool is always forced to
true/false
Jason
^ permalink raw reply
* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Mathieu Desnoyers @ 2019-06-24 17:50 UTC (permalink / raw)
To: Joel Fernandes, Google
Cc: Peter Zijlstra, Frank Ch. Eigler, 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,
Ard Biesheuvel, Thomas Gleixner, oprofile-list, netdev, bpf
In-Reply-To: <20190624155213.GB261936@google.com>
----- On Jun 24, 2019, at 11:52 AM, Joel Fernandes, Google joel@joelfernandes.org wrote:
> On Mon, Jun 24, 2019 at 10:01:04AM -0400, Mathieu Desnoyers wrote:
>> ----- On Jun 24, 2019, at 5:18 AM, Peter Zijlstra peterz@infradead.org wrote:
>>
>> > While auditing all module notifiers I noticed a whole bunch of fail
>> > wrt the return value. Notifiers have a 'special' return semantics.
>> >
>> > Cc: Robert Richter <rric@kernel.org>
>> > Cc: Steven Rostedt <rostedt@goodmis.org>
>> > Cc: Ingo Molnar <mingo@redhat.com>
>> > Cc: Alexei Starovoitov <ast@kernel.org>
>> > Cc: Daniel Borkmann <daniel@iogearbox.net>
>> > Cc: Martin KaFai Lau <kafai@fb.com>
>> > Cc: Song Liu <songliubraving@fb.com>
>> > Cc: Yonghong Song <yhs@fb.com>
>> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
>> > Cc: "Paul E. McKenney" <paulmck@linux.ibm.com>
>> > Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org>
>> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: oprofile-list@lists.sf.net
>> > Cc: linux-kernel@vger.kernel.org
>> > Cc: netdev@vger.kernel.org
>> > Cc: bpf@vger.kernel.org
>> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>
>> Thanks Peter for looking into this, especially considering your
>> endless love for kernel modules! ;)
>>
>> It's not directly related to your changes, but I notice that
>> kernel/trace/trace_printk.c:hold_module_trace_bprintk_format()
>> appears to leak memory. Am I missing something ?
>
> Could you elaborate? Do you mean there is no MODULE_STATE_GOING notifier
> check? If that's what you mean then I agree, there should be some place
> where the format structures are freed when the module is unloaded no?
Yes, the lack of GOING notifier is worrying considering that GOING
performs memory allocation.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH net-next 1/3] net/sched: Introduce action ct
From: Cong Wang @ 2019-06-24 17:46 UTC (permalink / raw)
To: Paul Blakey
Cc: Marcelo Ricardo Leitner, Toke Høiland-Jørgensen,
Jiri Pirko, Roi Dayan, Yossi Kuperman, Oz Shlomo,
netdev@vger.kernel.org, David Miller, Aaron Conole, Zhike Wang,
Rony Efraim, nst-kernel@redhat.com, John Hurley, Simon Horman,
Justin Pettit, Kevin Darbyshire-Bryant
In-Reply-To: <db10725e-d31a-efda-e57e-9978fd680c92@mellanox.com>
On Thu, Jun 20, 2019 at 12:32 AM Paul Blakey <paulb@mellanox.com> wrote:
>
>
> On 6/18/2019 7:03 PM, Cong Wang wrote:
> > On Fri, Jun 14, 2019 at 12:24 PM Marcelo Ricardo Leitner
> > <marcelo.leitner@gmail.com> wrote:
> >> On Fri, Jun 14, 2019 at 11:07:37AM -0700, Cong Wang wrote:
> >>> On Tue, Jun 11, 2019 at 9:44 AM Marcelo Ricardo Leitner
> >>> <marcelo.leitner@gmail.com> wrote:
> >>>> I had suggested to let act_ct handle the above as well, as there is a
> >>>> big chunk of code on both that is pretty similar. There is quite some
> >>>> boilerplate for interfacing with conntrack which is duplicated.
> >>> Why do you want to mix retrieving conntrack info with executing
> >>> conntrack?
> >> To save on the heavy boilerplate for interfacing with conntrack.
> >>
> >>> They are totally different things to me, act_ctinfo merely retrieves
> >>> information from conntrack, while this one, act_ct, is supposed to
> >>> move packets to conntrack.
> >> Seems we have a different understanding for "move packets to
> >> conntrack": conntrack will not consume the packets after this.
> >> But after act_ct is executed, if not with the clear flag, skb will now
> >> have the skb->_nfct entry available, on which flower then will be able
> >> to match. So in essence, it is also fetching information from
> >> conntrack.
> > Interesting. Is it because cls_flower uses conntrack for flow dissection?
> > What's the reason behind?
> >
> > Again, I am still not convinced to do L3 operations in L2, skb->_nfct
> > belongs to conntrack which is L3, no matter the packet is consumed
> > or not.
> >
> > Thanks.
>
> I'm not sure what you mean, the reason behind what?
Yes, which should be the most important info in changelog.
>
> We use conntrack to track, mark the packet with conntrack info, and
> execute nat, then we push the
>
> headers back to continue processing the next action. This action will
> probably be followed by
>
> goto chain or reclassify and then cls_flower can be used to match on
> conntrack state and metadata via the new flow dissector change.
>
Sounds cool, but again why do we have to do this in L2?
Also, I am not sure if cls_flower really matches packets with any
conntrack state, from my quick glance of its code. Is this feature
merged in upstream?
Is this for ingress only? For egress, packets coming down from L3
so they should already have conntrack state as long as it is enabled?
Sorry for asking many questions here, because your changelog is too
short. :-/
Thanks.
^ permalink raw reply
* Re: [PATCH v7 0/6] Add support for Orange Pi 3
From: Ondřej Jirman @ 2019-06-24 17:46 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.102927.1268781741493594465.davem@davemloft.net>
On Mon, Jun 24, 2019 at 10:29:27AM -0700, David Miller wrote:
> From: megous@megous.com
> Date: Thu, 20 Jun 2019 15:47:42 +0200
>
> > From: Ondrej Jirman <megous@megous.com>
> >
> > This series implements support for Xunlong Orange Pi 3 board.
> >
> > - ethernet support (patches 1-3)
> > - HDMI support (patches 4-6)
> >
> > For some people, ethernet doesn't work after reboot (but works on cold
> > boot), when the stmmac driver is built into the kernel. It works when
> > the driver is built as a module. It's either some timing issue, or power
> > supply issue or a combination of both. Module build induces a power
> > cycling of the phy.
> >
> > I encourage people with this issue, to build the driver into the kernel,
> > and try to alter the reset timings for the phy in DTS or
> > startup-delay-us and report the findings.
>
> This is a mixture of networking and non-networking changes so it really
> can't go through my tree.
>
> I wonder how you expect this series to be merged?
>
> Thanks.
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 and regards,
o.
^ permalink raw reply
* RE: [net-next v2] tipc: add loopback device tracking
From: Jon Maloy @ 2019-06-24 17:45 UTC (permalink / raw)
To: David Miller, John Rutherford; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190624.072918.1626161455453937687.davem@davemloft.net>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of David Miller
> Sent: 24-Jun-19 10:29
> To: John Rutherford <john.rutherford@dektech.com.au>
> Cc: netdev@vger.kernel.org
> Subject: Re: [net-next v2] tipc: add loopback device tracking
>
> From: john.rutherford@dektech.com.au
> Date: Mon, 24 Jun 2019 16:44:35 +1000
>
> > Since node internal messages are passed directly to socket it is not
> > possible to observe this message exchange via tcpdump or wireshark.
> >
> > We now remedy this by making it possible to clone such messages and
> > send the clones to the loopback interface. The clones are dropped at
> > reception and have no functional role except making the traffic visible.
> >
> > The feature is turned on/off by enabling/disabling the loopback "bearer"
> > "eth:lo".
> >
> > Acked-by: Jon Maloy <jon.maloy@ericsson.com>
> > Signed-off-by: John Rutherford <john.rutherford@dektech.com.au>
>
> What a waste, just clone the packet, attach loopback to it, and go:
>
> if (dev_nit_active(loopback_dev))
> dev_queue_xmit_nit(skb, loopback_dev);
I was never quite happy with this patch, so thank you for the feedback!
///jon
^ permalink raw reply
* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nick Desaulniers @ 2019-06-24 17:37 UTC (permalink / raw)
To: David Miller, Nicolas Dichtel; +Cc: netdev, kbuild test robot
In-Reply-To: <20190624.102212.4398258272798722.davem@davemloft.net>
On Mon, Jun 24, 2019 at 10:22 AM David Miller <davem@davemloft.net> wrote:
>
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Mon, 24 Jun 2019 10:17:03 -0700
>
> > On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote:
> >> And you mean just changing to 'const' fixes something, how?
> >
> > See the warning in the above link (assuming now you have access).
> > Assigning a non-const variable the result of a function call that
> > returns const discards the const qualifier.
>
> Ok thanks for clarifying.
>
> However I was speaking in terms of this fixing a functional bug rather
> than a loss of const warning.
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.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* [PATCH v3 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds reference counting support for
alloc/free mac filters
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 6 +++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 12 +++--
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 72 +++++++++++++++++++++++++
3 files changed, 87 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 6260240743d5..1fbb640e896a 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1915,6 +1915,12 @@ int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
int *tcam_idx, const u8 *addr,
bool persistent, u8 *smt_idx);
+int cxgb4_alloc_mac_filt(struct adapter *adap, unsigned int viid,
+ bool free, unsigned int naddr,
+ const u8 **addr, u16 *idx,
+ u64 *hash, bool sleep_ok);
+int cxgb4_free_mac_filt(struct adapter *adap, unsigned int viid,
+ unsigned int naddr, const u8 **addr, bool sleep_ok);
int cxgb4_init_mps_ref_entries(struct adapter *adap);
void cxgb4_free_mps_ref_entries(struct adapter *adap);
int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1520e5294289..b08efc48d42f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -366,13 +366,19 @@ static int cxgb4_mac_sync(struct net_device *netdev, const u8 *mac_addr)
int ret;
u64 mhash = 0;
u64 uhash = 0;
+ /* idx stores the index of allocated filters,
+ * its size should be modified based on the number of
+ * MAC addresses that we allocate filters for
+ */
+
+ u16 idx[1] = {};
bool free = false;
bool ucast = is_unicast_ether_addr(mac_addr);
const u8 *maclist[1] = {mac_addr};
struct hash_mac_addr *new_entry;
- ret = t4_alloc_mac_filt(adap, adap->mbox, pi->viid, free, 1, maclist,
- NULL, ucast ? &uhash : &mhash, false);
+ ret = cxgb4_alloc_mac_filt(adap, pi->viid, free, 1, maclist,
+ idx, ucast ? &uhash : &mhash, false);
if (ret < 0)
goto out;
/* if hash != 0, then add the addr to hash addr list
@@ -410,7 +416,7 @@ static int cxgb4_mac_unsync(struct net_device *netdev, const u8 *mac_addr)
}
}
- ret = t4_free_mac_filt(adap, adap->mbox, pi->viid, 1, maclist, false);
+ ret = cxgb4_free_mac_filt(adap, pi->viid, 1, maclist, false);
return ret < 0 ? -EINVAL : 0;
}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index 067217c6ca05..b1a073eea60b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -3,6 +3,31 @@
#include "cxgb4.h"
+static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
+ const u8 *addr, const u8 *mask)
+{
+ u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
+ struct mps_entries_ref *mps_entry, *tmp;
+ int ret = -EINVAL;
+
+ spin_lock_bh(&adap->mps_ref_lock);
+ list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
+ if (ether_addr_equal(mps_entry->addr, addr) &&
+ ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
+ if (!refcount_dec_and_test(&mps_entry->refcnt)) {
+ spin_unlock_bh(&adap->mps_ref_lock);
+ return -EBUSY;
+ }
+ list_del(&mps_entry->list);
+ kfree(mps_entry);
+ ret = 0;
+ break;
+ }
+ }
+ spin_unlock_bh(&adap->mps_ref_lock);
+ return ret;
+}
+
static int cxgb4_mps_ref_dec(struct adapter *adap, u16 idx)
{
struct mps_entries_ref *mps_entry, *tmp;
@@ -54,6 +79,53 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_free_mac_filt(struct adapter *adap, unsigned int viid,
+ unsigned int naddr, const u8 **addr, bool sleep_ok)
+{
+ int ret, i;
+
+ for (i = 0; i < naddr; i++) {
+ if (!cxgb4_mps_ref_dec_by_mac(adap, addr[i], NULL)) {
+ ret = t4_free_mac_filt(adap, adap->mbox, viid,
+ 1, &addr[i], sleep_ok);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ /* return number of filters freed */
+ return naddr;
+}
+
+int cxgb4_alloc_mac_filt(struct adapter *adap, unsigned int viid,
+ bool free, unsigned int naddr, const u8 **addr,
+ u16 *idx, u64 *hash, bool sleep_ok)
+{
+ int ret, i;
+
+ ret = t4_alloc_mac_filt(adap, adap->mbox, viid, free,
+ naddr, addr, idx, hash, sleep_ok);
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < naddr; i++) {
+ if (idx[i] != 0xffff) {
+ if (cxgb4_mps_ref_inc(adap, addr[i], idx[i], NULL)) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ }
+ }
+
+ goto out;
+error:
+ cxgb4_free_mac_filt(adap, viid, naddr, addr, sleep_ok);
+
+out:
+ /* Returns a negative error number or the number of filters allocated */
+ return ret;
+}
+
int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
int *tcam_idx, const u8 *addr,
bool persistent, u8 *smt_idx)
--
2.12.0
^ permalink raw reply related
* [PATCH v3 net-next 3/4] cxgb4: Add MPS TCAM refcounting for cxgb4 change mac
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds TCAM reference counting
support for cxgb4 change mac path
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 7 +++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 14 +++++++-------
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 15 +++++++++++++++
3 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index c7ab57fd03be..6260240743d5 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1911,6 +1911,10 @@ int cxgb4_set_msix_aff(struct adapter *adap, unsigned short vec,
cpumask_var_t *aff_mask, int idx);
void cxgb4_clear_msix_aff(unsigned short vec, cpumask_var_t aff_mask);
+int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx);
+
int cxgb4_init_mps_ref_entries(struct adapter *adap);
void cxgb4_free_mps_ref_entries(struct adapter *adap);
int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
@@ -1935,5 +1939,8 @@ int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
u8 lookup_type,
u8 port_id,
bool sleep_ok);
+int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx);
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 4632827f05ba..1520e5294289 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -449,9 +449,9 @@ static int set_rxmode(struct net_device *dev, int mtu, bool sleep_ok)
* Addresses are programmed to hash region, if tcam runs out of entries.
*
*/
-static int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
- int *tcam_idx, const u8 *addr, bool persist,
- u8 *smt_idx)
+int cxgb4_change_mac(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr, bool persist,
+ u8 *smt_idx)
{
struct adapter *adapter = pi->adapter;
struct hash_mac_addr *entry, *new_entry;
@@ -505,8 +505,8 @@ static int link_start(struct net_device *dev)
ret = t4_set_rxmode(pi->adapter, mb, pi->viid, dev->mtu, -1, -1, -1,
!!(dev->features & NETIF_F_HW_VLAN_CTAG_RX), true);
if (ret == 0)
- ret = cxgb4_change_mac(pi, pi->viid, &pi->xact_addr_filt,
- dev->dev_addr, true, &pi->smt_idx);
+ ret = cxgb4_update_mac_filt(pi, pi->viid, &pi->xact_addr_filt,
+ dev->dev_addr, true, &pi->smt_idx);
if (ret == 0)
ret = t4_link_l1cfg(pi->adapter, mb, pi->tx_chan,
&pi->link_cfg);
@@ -3020,8 +3020,8 @@ static int cxgb_set_mac_addr(struct net_device *dev, void *p)
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
- ret = cxgb4_change_mac(pi, pi->viid, &pi->xact_addr_filt,
- addr->sa_data, true, &pi->smt_idx);
+ ret = cxgb4_update_mac_filt(pi, pi->viid, &pi->xact_addr_filt,
+ addr->sa_data, true, &pi->smt_idx);
if (ret < 0)
return ret;
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index b942748c7dfa..067217c6ca05 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -54,6 +54,21 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_update_mac_filt(struct port_info *pi, unsigned int viid,
+ int *tcam_idx, const u8 *addr,
+ bool persistent, u8 *smt_idx)
+{
+ int ret;
+
+ ret = cxgb4_change_mac(pi, viid, tcam_idx,
+ addr, persistent, smt_idx);
+ if (ret < 0)
+ return ret;
+
+ cxgb4_mps_ref_inc(pi->adapter, addr, *tcam_idx, NULL);
+ return ret;
+}
+
int cxgb4_free_raw_mac_filt(struct adapter *adap,
unsigned int viid,
const u8 *addr,
--
2.12.0
^ permalink raw reply related
* [PATCH v3 net-next 2/4] cxgb4: Add MPS TCAM refcounting for raw mac filters
From: Raju Rangoju @ 2019-06-24 17:35 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, dt, rajur
In-Reply-To: <20190624173535.12572-1-rajur@chelsio.com>
This patch adds TCAM reference counting
support for raw mac filters.
Signed-off-by: Raju Rangoju <rajur@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 16 +++++++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c | 46 ++++++++++++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 39ccd4c64d48..c7ab57fd03be 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -1919,5 +1919,21 @@ int cxgb4_alloc_encap_mac_filt(struct adapter *adap, unsigned int viid,
u8 dip_hit, u8 lookup_type, bool sleep_ok);
int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
int idx, bool sleep_ok);
+int cxgb4_free_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok);
+int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok);
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
index b8a5375bf64d..b942748c7dfa 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c
@@ -54,6 +54,52 @@ static int cxgb4_mps_ref_inc(struct adapter *adap, const u8 *mac_addr,
return ret;
}
+int cxgb4_free_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok)
+{
+ int ret = 0;
+
+ if (!cxgb4_mps_ref_dec(adap, idx))
+ ret = t4_free_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+
+ return ret;
+}
+
+int cxgb4_alloc_raw_mac_filt(struct adapter *adap,
+ unsigned int viid,
+ const u8 *addr,
+ const u8 *mask,
+ unsigned int idx,
+ u8 lookup_type,
+ u8 port_id,
+ bool sleep_ok)
+{
+ int ret;
+
+ ret = t4_alloc_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+ if (ret < 0)
+ return ret;
+
+ if (cxgb4_mps_ref_inc(adap, addr, ret, mask)) {
+ ret = -ENOMEM;
+ t4_free_raw_mac_filt(adap, viid, addr,
+ mask, idx, lookup_type,
+ port_id, sleep_ok);
+ }
+
+ return ret;
+}
+
int cxgb4_free_encap_mac_filt(struct adapter *adap, unsigned int viid,
int idx, bool sleep_ok)
{
--
2.12.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox