* Re: [PATCH net-next 0/2] net: mediatek: Add MT7621 TRGMII mode support
From: René van Dorst @ 2019-06-18 11:46 UTC (permalink / raw)
To: Florian Fainelli
Cc: Andrew Lunn, Sean Wang, David S . Miller, Matthias Brugger,
Vivien Didelot, netdev, john, linux-mediatek, linux-mips
In-Reply-To: <7f2fc770-1787-72f8-b91d-e2b12e74d39e@gmail.com>
Quoting Florian Fainelli <f.fainelli@gmail.com>:
Hi Andrew and Florian,
> On 6/17/2019 6:53 PM, Andrew Lunn wrote:
>>> By adding some extra speed states in the code it seems to work.
>>>
>>> + if (state->speed == 1200)
>>> + mcr |= PMCR_FORCE_SPEED_1000;
>>
>> Hi René
>>
>> Is TRGMII always 1.2G? Or can you set it to 1000 or 1200?
In case of the MT7621 SOC yes, according to the SDKs the MT7623 has 2 options
2GBit and 2.6Gbit. The current mt7530 driver only set TRGMII speed at 2Gbit.
>> This PMCR_FORCE_SPEED_1000 feels wrong.
>
> It is not uncommon to have to "force" 1G to get a higher speed, there is
> something similar with B53 switches configuring the CPU ports at 2GB/sec
> (proprietary too and not standardized either).
On the SOC MAC side it is basicly only a MAC clock change.
MAC control registers still need to be set forced 1G.
>
>>
>>>> We could consider adding 1200BaseT/Full?
>>>
>>> I don't have any opinion about this.
>>> It is great that it shows nicely in ethtool but I think supporting more
>>> speeds in phy_speed_to_str() is enough.
>>>
>>> Also you may want to add other SOCs trgmii ranges too:
>>> - 1200BaseT/Full for mt7621 only
>>> - 2000BaseT/Full for mt7623 and mt7683
>>> - 2600BaseT/Full for mt7623 only
>>
>> Are these standardised in any way? Or MTK proprietary? Also, is the T
>> in BaseT correct? These speeds work over copper cables? Or should we
>> be talking about 1200BaseKX?
>
> Looks like this is MTK proprietary:
>
> http://lists.infradead.org/pipermail/linux-mediatek/2016-September/007083.html
> https://patchwork.kernel.org/patch/9341129/
> --
> Florian
MTK proprietary, But I think it is equal too the RGMII but with a
faster clock.
But do we need a "xxxxBaseT/Full" at all for these fixed-link cases?
If I am correct a "xxxxBaseT/Full" is only needed to automatically select the
best option. But with fixed-link we force it so extra "xxxxBaseT/Full" is not
needed.
Greats,
René
^ permalink raw reply
* Re: [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
From: Ulf Hansson @ 2019-06-18 11:40 UTC (permalink / raw)
To: Kalle Valo
Cc: Douglas Anderson, Adrian Hunter, Arend van Spriel,
brcm80211-dev-list.pdl, open list:ARM/Rockchip SoC..., Double Lo,
Brian Norris, linux-wireless, Naveen Gupta, Madhan Mohan R,
Matthias Kaehlcke, Wright Feng, Chi-Hsien Lin, netdev,
brcm80211-dev-list, YueHaibing, Allison Randal, Thomas Gleixner,
Hante Meuleman, Greg Kroah-Hartman, Niklas Söderlund,
Ritesh Harjani, Michael Trimarchi, Wolfram Sang, Franky Lin,
Ondrej Jirman, Jiong Wu, David S. Miller,
linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Avri Altman
In-Reply-To: <87v9x39mxf.fsf@kamboji.qca.qualcomm.com>
On Tue, 18 Jun 2019 at 13:02, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> Ulf Hansson <ulf.hansson@linaro.org> writes:
>
> > On Mon, 17 Jun 2019 at 19:57, Douglas Anderson <dianders@chromium.org> wrote:
> >>
> >> This series attempts to deal better with the expected transmission
> >> errors related to the idle states (handled by the Always-On-Subsystem
> >> or AOS) on the SDIO-based WiFi on rk3288-veyron-minnie,
> >> rk3288-veyron-speedy, and rk3288-veyron-mickey.
> >>
> >> Some details about those errors can be found in
> >> <https://crbug.com/960222>, but to summarize it here: if we try to
> >> send the wakeup command to the WiFi card at the same time it has
> >> decided to wake up itself then it will behave badly on the SDIO bus.
> >> This can cause timeouts or CRC errors.
> >>
> >> When I tested on 4.19 and 4.20 these CRC errors can be seen to cause
> >> re-tuning. Since I am currently developing on 4.19 this was the
> >> original problem I attempted to solve.
> >>
> >> On mainline it turns out that you don't see the retuning errors but
> >> you see tons of spam about timeouts trying to wakeup from sleep. I
> >> tracked down the commit that was causing that and have partially
> >> reverted it here. I have no real knowledge about Broadcom WiFi, but
> >> the commit that was causing problems sounds (from the descriptioin) to
> >> be a hack commit penalizing all Broadcom WiFi users because of a bug
> >> in a Cypress SD controller. I will let others comment if this is
> >> truly the case and, if so, what the right solution should be.
> >>
> >> For v3 of this series I have added 2 patches to the end of the series
> >> to address errors that would show up on systems with these same SDIO
> >> WiFi cards when used on controllers that do periodic retuning. These
> >> systems need an extra fix to prevent the retuning from happening when
> >> the card is asleep.
> >>
> >> I believe v5 of this series is all ready to go assuming Kalle Valo is
> >> good with it. I've added after-the-cut notes to patches awaiting his
> >> Ack and have added other tags collected so far.
> >>
> >> Changes in v5:
> >> - Add missing sdio_retune_crc_enable() in comments (Ulf).
> >> - /s/reneable/re-enable (Ulf).
> >> - Remove leftover prototypes: mmc_expect_errors_begin() / end() (Ulf).
> >> - Rewording of "sleep command" in commit message (Arend).
> >>
> >> Changes in v4:
> >> - Moved to SDIO API only (Adrian, Ulf).
> >> - Renamed to make it less generic, now retune_crc_disable (Ulf).
> >> - Function header makes it clear host must be claimed (Ulf).
> >> - No more WARN_ON (Ulf).
> >> - Adjust to API rename (Adrian, Ulf).
> >> - Moved retune hold/release to SDIO API (Adrian).
> >> - Adjust to API rename (Adrian).
> >>
> >> Changes in v3:
> >> - Took out the spinlock since I believe this is all in one context.
> >> - Expect errors for all of brcmf_sdio_kso_control() (Adrian).
> >> - ("mmc: core: Export mmc_retune_hold_now() mmc_retune_release()") new for v3.
> >> - ("brcmfmac: sdio: Don't tune while the card is off") new for v3.
> >>
> >> Changes in v2:
> >> - A full revert, not just a partial one (Arend). ...with explicit Cc.
> >> - Updated commit message to clarify based on discussion of v1.
> >>
> >> Douglas Anderson (5):
> >> Revert "brcmfmac: disable command decode in sdio_aos"
> >> mmc: core: API to temporarily disable retuning for SDIO CRC errors
> >> brcmfmac: sdio: Disable auto-tuning around commands expected to fail
> >> mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
> >> brcmfmac: sdio: Don't tune while the card is off
> >>
> >> drivers/mmc/core/core.c | 5 +-
> >> drivers/mmc/core/sdio_io.c | 77 +++++++++++++++++++
> >> .../broadcom/brcm80211/brcmfmac/sdio.c | 17 ++--
> >> include/linux/mmc/host.h | 1 +
> >> include/linux/mmc/sdio_func.h | 6 ++
> >> 5 files changed, 99 insertions(+), 7 deletions(-)
> >>
> >> --
> >> 2.22.0.410.gd8fdbe21b5-goog
> >>
> >
> > Applied for fixes, thanks!
> >
> > Some minor changes:
> > 1) Dropped the a few "commit notes", that was more related to version
> > and practical information about the series.
> > 2) Dropped fixes tags for patch 2->5, but instead put a stable tag
> > targeted for v4.18+.
> >
> > Awaiting an ack from Kalle before sending the PR to Linus.
> >
> > Kalle, perhaps you prefer to pick patch 1, as it could go separate.
> > Then please tell - and/or if there is anything else you want me to
> > change.
>
> TBH I haven't followed the thread (or patches) that closely :) So feel
> free to take them and push them to Linus.
>
I take that as an ack and will add your tag for it, thanks!
Kind regards
Uffe
^ permalink raw reply
* 4.19: udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
From: Naresh Kamboju @ 2019-06-18 11:27 UTC (permalink / raw)
To: David S. Miller, Netdev, open list,
open list:KERNEL SELFTEST FRAMEWORK, Willem de Bruijn, fklassen
selftests: net: udpgso_bench.sh failed on 4.19, 4.14, 4.9 and 4.4 branches.
PASS on stable branch 5.1, mainline and next.
This failure is started happening on 4.19 and older kernel branches after
kselftest upgrade to version 5.1
Is there any possibilities to backport ?
Error:
udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
Test output:
-----------------
selftests: net: udpgso_bench.sh
ipv4
tcp
tcp rx: 469 MB/s 7930 calls/s
tcp tx: 469 MB/s 7961 calls/s 7961 msg/s
tcp rx: 470 MB/s 7941 calls/s
tcp tx: 470 MB/s 7977 calls/s 7977 msg/s
tcp rx: 470 MB/s 7933 calls/s
tcp tx: 470 MB/s 7975 calls/s 7975 msg/s
tcp zerocopy
tcp tx: 357 MB/s 6064 calls/s 6064 msg/s
tcp rx: 357 MB/s 6052 calls/s
tcp tx: 352 MB/s 5981 calls/s 5981 msg/s
tcp rx: 352 MB/s 5979 calls/s
tcp tx: 350 MB/s 5937 calls/s 5937 msg/s
tcp rx: 350 MB/s 5938 calls/s
udp
udp rx: 23 MB/s 16505 calls/s
udp tx: 23 MB/s 16464 calls/s 392 msg/s
udp rx: 23 MB/s 16500 calls/s
udp tx: 23 MB/s 16506 calls/s 393 msg/s
udp rx: 23 MB/s 16396 calls/s
udp gso
udp rx: 536 MB/s 9097 calls/s
udp tx: 545 MB/s 9246 calls/s 9246 msg/s
udp rx: 545 MB/s 9256 calls/s
udp tx: 545 MB/s 9256 calls/s 9256 msg/s
udp rx: 545 MB/s 9259 calls/s
udp tx: 545 MB/s 9258 calls/s 9258 msg/s
udp rx: 545 MB/s 9252 calls/s
udp gso zerocopy
./udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
ipv6
tcp
tcp tx: 470 MB/s 7979 calls/s 7979 msg/s
tcp rx: 470 MB/s 7947 calls/s
tcp rx: 471 MB/s 7979 calls/s
tcp tx: 514 MB/s 8721 calls/s 8721 msg/s
tcp zerocopy
tcp tx: 392 MB/s 6658 calls/s 6658 msg/s
tcp rx: 392 MB/s 6399 calls/s
tcp rx: 350 MB/s 5936 calls/s
tcp tx: 350 MB/s 5945 calls/s 5945 msg/s
tcp rx: 350 MB/s 5937 calls/s
tcp tx: 350 MB/s 5940 calls/s 5940 msg/s
udp
udp rx: 20 MB/s 14802 calls/s
udp tx: 20 MB/s 14921 calls/s 347 msg/s
udp rx: 24 MB/s 17797 calls/s
udp tx: 24 MB/s 17802 calls/s 414 msg/s
udp rx: 17 MB/s 12453 calls/s
udp tx: 17 MB/s 12470 calls/s 290 msg/s
udp rx: 17 MB/s 12409 calls/s
udp tx: 545 MB/s 9257 calls/s 9257 msg/s
udp rx: 545 MB/s 9249 calls/s
udp tx: 545 MB/s 9248 calls/s 9248 msg/s
udp rx: 545 MB/s 9254 calls/s
udp tx: 545 MB/s 9254 calls/s 9254 msg/s
udp rx: 545 MB/s 9260 calls/s
udp gso zerocopy
./udpgso_bench_tx: setsockopt zerocopy: Unknown error 524
not ok 1.. selftests: net: udpgso_bench.sh [FAIL]
selftests: net_udpgso_bench.sh [FAIL]
Best regards
Naresh Kamboju
^ permalink raw reply
* [PATCH] [v2] ipsec: select crypto ciphers for xfrm_algo
From: Arnd Bergmann @ 2019-06-18 11:22 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, David S. Miller
Cc: Arnd Bergmann, Florian Westphal, netdev, linux-kernel
kernelci.org reports failed builds on arc because of what looks
like an old missed 'select' statement:
net/xfrm/xfrm_algo.o: In function `xfrm_probe_algs':
xfrm_algo.c:(.text+0x1e8): undefined reference to `crypto_has_ahash'
I don't see this in randconfig builds on other architectures, but
it's fairly clear we want to select the hash code for it, like we
do for all its other users. As Herbert points out, CRYPTO_BLKCIPHER
is also required even though it has not popped up in build tests.
Fixes: 17bc19702221 ("ipsec: Use skcipher and ahash when probing algorithms")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
net/xfrm/Kconfig | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/xfrm/Kconfig b/net/xfrm/Kconfig
index c967fc3c38c8..51bb6018f3bf 100644
--- a/net/xfrm/Kconfig
+++ b/net/xfrm/Kconfig
@@ -15,6 +15,8 @@ config XFRM_ALGO
tristate
select XFRM
select CRYPTO
+ select CRYPTO_HASH
+ select CRYPTO_BLKCIPHER
if INET
config XFRM_USER
--
2.20.0
^ permalink raw reply related
* [PATCH] [v2] net/mlx5e: reduce stack usage in mlx5_eswitch_termtbl_create
From: Arnd Bergmann @ 2019-06-18 11:15 UTC (permalink / raw)
To: Saeed Mahameed, Leon Romanovsky
Cc: Arnd Bergmann, Mark Bloch, Oz Shlomo, Paul Blakey, Eli Britstein,
David S. Miller, Or Gerlitz, Maor Gottlieb, netdev, linux-rdma,
linux-kernel
Putting an empty 'mlx5_flow_spec' structure on the stack is a bit
wasteful and causes a warning on 32-bit architectures when building
with clang -fsanitize-coverage:
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c: In function 'mlx5_eswitch_termtbl_create':
drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c:90:1: error: the frame size of 1032 bytes is larger than 1024 bytes [-Werror=frame-larger-than=]
Since the structure is never written to, we can statically allocate
it to avoid the stack usage. To be on the safe side, mark all
subsequent function arguments that we pass it into as 'const'
as well.
Fixes: 10caabdaad5a ("net/mlx5e: Use termination table for VLAN push actions")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Saeed Mahameed <saeedm@mellanox.com>
Acked-by: Mark Bloch <markb@mellanox.com>
---
v2: only style changes (reverse xmas tree)
---
.../mlx5/core/eswitch_offloads_termtbl.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/fs_core.c | 20 +++++++++----------
include/linux/mlx5/fs.h | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
index cb7d8ebe2c95..1d55a324a17e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads_termtbl.c
@@ -49,8 +49,8 @@ mlx5_eswitch_termtbl_create(struct mlx5_core_dev *dev,
struct mlx5_termtbl_handle *tt,
struct mlx5_flow_act *flow_act)
{
+ static const struct mlx5_flow_spec spec = {};
struct mlx5_flow_namespace *root_ns;
- struct mlx5_flow_spec spec = {};
int prio, flags;
int err;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
index fe76c6fd6d80..739123e1363b 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fs_core.c
@@ -584,7 +584,7 @@ static int insert_fte(struct mlx5_flow_group *fg, struct fs_fte *fte)
}
static struct fs_fte *alloc_fte(struct mlx5_flow_table *ft,
- u32 *match_value,
+ const u32 *match_value,
struct mlx5_flow_act *flow_act)
{
struct mlx5_flow_steering *steering = get_steering(&ft->node);
@@ -612,7 +612,7 @@ static void dealloc_flow_group(struct mlx5_flow_steering *steering,
static struct mlx5_flow_group *alloc_flow_group(struct mlx5_flow_steering *steering,
u8 match_criteria_enable,
- void *match_criteria,
+ const void *match_criteria,
int start_index,
int end_index)
{
@@ -642,7 +642,7 @@ static struct mlx5_flow_group *alloc_flow_group(struct mlx5_flow_steering *steer
static struct mlx5_flow_group *alloc_insert_flow_group(struct mlx5_flow_table *ft,
u8 match_criteria_enable,
- void *match_criteria,
+ const void *match_criteria,
int start_index,
int end_index,
struct list_head *prev)
@@ -1285,7 +1285,7 @@ add_rule_fte(struct fs_fte *fte,
}
static struct mlx5_flow_group *alloc_auto_flow_group(struct mlx5_flow_table *ft,
- struct mlx5_flow_spec *spec)
+ const struct mlx5_flow_spec *spec)
{
struct list_head *prev = &ft->node.children;
struct mlx5_flow_group *fg;
@@ -1451,7 +1451,7 @@ static int check_conflicting_ftes(struct fs_fte *fte, const struct mlx5_flow_act
}
static struct mlx5_flow_handle *add_rule_fg(struct mlx5_flow_group *fg,
- u32 *match_value,
+ const u32 *match_value,
struct mlx5_flow_act *flow_act,
struct mlx5_flow_destination *dest,
int dest_num,
@@ -1536,7 +1536,7 @@ static void free_match_list(struct match_list_head *head)
static int build_match_list(struct match_list_head *match_head,
struct mlx5_flow_table *ft,
- struct mlx5_flow_spec *spec)
+ const struct mlx5_flow_spec *spec)
{
struct rhlist_head *tmp, *list;
struct mlx5_flow_group *g;
@@ -1589,7 +1589,7 @@ static u64 matched_fgs_get_version(struct list_head *match_head)
static struct fs_fte *
lookup_fte_locked(struct mlx5_flow_group *g,
- u32 *match_value,
+ const u32 *match_value,
bool take_write)
{
struct fs_fte *fte_tmp;
@@ -1622,7 +1622,7 @@ lookup_fte_locked(struct mlx5_flow_group *g,
static struct mlx5_flow_handle *
try_add_to_existing_fg(struct mlx5_flow_table *ft,
struct list_head *match_head,
- struct mlx5_flow_spec *spec,
+ const struct mlx5_flow_spec *spec,
struct mlx5_flow_act *flow_act,
struct mlx5_flow_destination *dest,
int dest_num,
@@ -1715,7 +1715,7 @@ try_add_to_existing_fg(struct mlx5_flow_table *ft,
static struct mlx5_flow_handle *
_mlx5_add_flow_rules(struct mlx5_flow_table *ft,
- struct mlx5_flow_spec *spec,
+ const struct mlx5_flow_spec *spec,
struct mlx5_flow_act *flow_act,
struct mlx5_flow_destination *dest,
int dest_num)
@@ -1823,7 +1823,7 @@ static bool fwd_next_prio_supported(struct mlx5_flow_table *ft)
struct mlx5_flow_handle *
mlx5_add_flow_rules(struct mlx5_flow_table *ft,
- struct mlx5_flow_spec *spec,
+ const struct mlx5_flow_spec *spec,
struct mlx5_flow_act *flow_act,
struct mlx5_flow_destination *dest,
int num_dest)
diff --git a/include/linux/mlx5/fs.h b/include/linux/mlx5/fs.h
index 2ddaa97f2179..c0c029664527 100644
--- a/include/linux/mlx5/fs.h
+++ b/include/linux/mlx5/fs.h
@@ -200,7 +200,7 @@ struct mlx5_flow_act {
*/
struct mlx5_flow_handle *
mlx5_add_flow_rules(struct mlx5_flow_table *ft,
- struct mlx5_flow_spec *spec,
+ const struct mlx5_flow_spec *spec,
struct mlx5_flow_act *flow_act,
struct mlx5_flow_destination *dest,
int num_dest);
--
2.20.0
^ permalink raw reply related
* Ihre Angebot
From: Oliver @ 2019-06-18 11:03 UTC (permalink / raw)
To: netdev
Sehr geehrte Damen und Herren,
wir haben Ihre Firmenwebseite besucht und wir sind der Meinung, sie sieht ausgezeichnet aus!
Wenn Ihnen daran liegt, neue Kunden zu gewinnen, dann haben wir für Sie ein Angebot.
Wir verfügen über den neusten, stets aktualisierten Firmenkatalog 2019. In wenigen Augenblicken können Sie die interessanten Branchen / Wirtschaftssektoren aussuchen und die Kontakte exportieren.
Alle unsere Kontakte entsprechen den GDPR-Regeln. Wir verfügen nur über Firmenangaben oder Firmenkontaktdaten.
--
1. Firmenadressen 2019 DE 1 Mio. Firmenadressen ( 1 457 620 ) - 190 EUR ( bis zum 18.06.2019 )
2. Gesamtpaket 2019 DE,AT,CH ( 1 747 921 ) - 240 EUR ( bis zum 18.06.2019 )
3. Europa 2019 DE, AT, CH, UK, FR, IT, ES ( 8 123 551 ) - 390 EUR ( bis zum 18.06.2019 )
--
Die Verwendungsmöglichkeiten der Datenbanken sind praktisch unbegrenzt und Sie können durch Verwendung der von uns entwickelten
Programme des personalisierten Versendens von Angeboten u.ä. mittels E-mailing bzw. Fax effektive und sichere Werbekampagnen damit durchführen.
Wir möchten Sie gerne dazu einladen, unsere Webseite zu besuchen:
http://www.acdbmarketing.net/?page=catalog
Mit freundlichen Grüßen
GMC - Team
^ permalink raw reply
* Re: [PATCH v5 0/5] brcmfmac: sdio: Deal better w/ transmission errors related to idle
From: Kalle Valo @ 2019-06-18 11:02 UTC (permalink / raw)
To: Ulf Hansson
Cc: Douglas Anderson, Adrian Hunter, Arend van Spriel,
brcm80211-dev-list.pdl, open list:ARM/Rockchip SoC..., Double Lo,
Brian Norris, linux-wireless, Naveen Gupta, Madhan Mohan R,
Matthias Kaehlcke, Wright Feng, Chi-Hsien Lin, netdev,
brcm80211-dev-list, YueHaibing, Allison Randal, Thomas Gleixner,
Hante Meuleman, Greg Kroah-Hartman, Niklas Söderlund,
Ritesh Harjani, Michael Trimarchi, Wolfram Sang, Franky Lin,
Ondrej Jirman, Jiong Wu, David S. Miller,
linux-mmc@vger.kernel.org, Linux Kernel Mailing List, Avri Altman
In-Reply-To: <CAPDyKFpaX6DSM_BjtghAHUf7qYCyEG+wMagXPUdgz3Eutovqfw@mail.gmail.com>
Ulf Hansson <ulf.hansson@linaro.org> writes:
> On Mon, 17 Jun 2019 at 19:57, Douglas Anderson <dianders@chromium.org> wrote:
>>
>> This series attempts to deal better with the expected transmission
>> errors related to the idle states (handled by the Always-On-Subsystem
>> or AOS) on the SDIO-based WiFi on rk3288-veyron-minnie,
>> rk3288-veyron-speedy, and rk3288-veyron-mickey.
>>
>> Some details about those errors can be found in
>> <https://crbug.com/960222>, but to summarize it here: if we try to
>> send the wakeup command to the WiFi card at the same time it has
>> decided to wake up itself then it will behave badly on the SDIO bus.
>> This can cause timeouts or CRC errors.
>>
>> When I tested on 4.19 and 4.20 these CRC errors can be seen to cause
>> re-tuning. Since I am currently developing on 4.19 this was the
>> original problem I attempted to solve.
>>
>> On mainline it turns out that you don't see the retuning errors but
>> you see tons of spam about timeouts trying to wakeup from sleep. I
>> tracked down the commit that was causing that and have partially
>> reverted it here. I have no real knowledge about Broadcom WiFi, but
>> the commit that was causing problems sounds (from the descriptioin) to
>> be a hack commit penalizing all Broadcom WiFi users because of a bug
>> in a Cypress SD controller. I will let others comment if this is
>> truly the case and, if so, what the right solution should be.
>>
>> For v3 of this series I have added 2 patches to the end of the series
>> to address errors that would show up on systems with these same SDIO
>> WiFi cards when used on controllers that do periodic retuning. These
>> systems need an extra fix to prevent the retuning from happening when
>> the card is asleep.
>>
>> I believe v5 of this series is all ready to go assuming Kalle Valo is
>> good with it. I've added after-the-cut notes to patches awaiting his
>> Ack and have added other tags collected so far.
>>
>> Changes in v5:
>> - Add missing sdio_retune_crc_enable() in comments (Ulf).
>> - /s/reneable/re-enable (Ulf).
>> - Remove leftover prototypes: mmc_expect_errors_begin() / end() (Ulf).
>> - Rewording of "sleep command" in commit message (Arend).
>>
>> Changes in v4:
>> - Moved to SDIO API only (Adrian, Ulf).
>> - Renamed to make it less generic, now retune_crc_disable (Ulf).
>> - Function header makes it clear host must be claimed (Ulf).
>> - No more WARN_ON (Ulf).
>> - Adjust to API rename (Adrian, Ulf).
>> - Moved retune hold/release to SDIO API (Adrian).
>> - Adjust to API rename (Adrian).
>>
>> Changes in v3:
>> - Took out the spinlock since I believe this is all in one context.
>> - Expect errors for all of brcmf_sdio_kso_control() (Adrian).
>> - ("mmc: core: Export mmc_retune_hold_now() mmc_retune_release()") new for v3.
>> - ("brcmfmac: sdio: Don't tune while the card is off") new for v3.
>>
>> Changes in v2:
>> - A full revert, not just a partial one (Arend). ...with explicit Cc.
>> - Updated commit message to clarify based on discussion of v1.
>>
>> Douglas Anderson (5):
>> Revert "brcmfmac: disable command decode in sdio_aos"
>> mmc: core: API to temporarily disable retuning for SDIO CRC errors
>> brcmfmac: sdio: Disable auto-tuning around commands expected to fail
>> mmc: core: Add sdio_retune_hold_now() and sdio_retune_release()
>> brcmfmac: sdio: Don't tune while the card is off
>>
>> drivers/mmc/core/core.c | 5 +-
>> drivers/mmc/core/sdio_io.c | 77 +++++++++++++++++++
>> .../broadcom/brcm80211/brcmfmac/sdio.c | 17 ++--
>> include/linux/mmc/host.h | 1 +
>> include/linux/mmc/sdio_func.h | 6 ++
>> 5 files changed, 99 insertions(+), 7 deletions(-)
>>
>> --
>> 2.22.0.410.gd8fdbe21b5-goog
>>
>
> Applied for fixes, thanks!
>
> Some minor changes:
> 1) Dropped the a few "commit notes", that was more related to version
> and practical information about the series.
> 2) Dropped fixes tags for patch 2->5, but instead put a stable tag
> targeted for v4.18+.
>
> Awaiting an ack from Kalle before sending the PR to Linus.
>
> Kalle, perhaps you prefer to pick patch 1, as it could go separate.
> Then please tell - and/or if there is anything else you want me to
> change.
TBH I haven't followed the thread (or patches) that closely :) So feel
free to take them and push them to Linus.
--
Kalle Valo
^ permalink raw reply
* RE: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport number in VF vports and uplink ingress ACLs
From: Parav Pandit @ 2019-06-18 11:00 UTC (permalink / raw)
To: Saeed Mahameed, Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Eli Britstein, Roi Dayan, Mark Bloch
In-Reply-To: <20190617192247.25107-6-saeedm@mellanox.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Saeed Mahameed
> Sent: Tuesday, June 18, 2019 12:53 AM
> To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> <leonro@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Jianbo Liu
> <jianbol@mellanox.com>; Eli Britstein <elibr@mellanox.com>; Roi Dayan
> <roid@mellanox.com>; Mark Bloch <markb@mellanox.com>
> Subject: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport
> number in VF vports and uplink ingress ACLs
>
> From: Jianbo Liu <jianbol@mellanox.com>
>
> When a dual-port VHCA sends a RoCE packet on its non-native port, and the
> packet arrives to its affiliated vport FDB, a mismatch might occur on the rules
> that match the packet source vport as it is not represented by single VHCA only
> in this case. So we change to match on metadata instead of source vport.
> To do that, a rule is created in all vports and uplink ingress ACLs, to save the
> source vport number and vhca id in the packet's metadata in order to match on
> it later.
> The metadata register used is the first of the 32-bit type C registers. It can be
> used for matching and header modify operations. The higher 16 bits of this
> register are for vhca id, and the lower 16 ones is for vport number.
> This change is not for dual-port RoCE only. If HW and FW allow, the vport
> metadata matching is enabled by default.
>
> Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> Reviewed-by: Eli Britstein <elibr@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +
> .../net/ethernet/mellanox/mlx5/core/eswitch.h | 9 +
> .../mellanox/mlx5/core/eswitch_offloads.c | 183 ++++++++++++++----
> include/linux/mlx5/eswitch.h | 3 +
> 4 files changed, 161 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index a42a23e505df..1235fd84ae3a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1168,6 +1168,8 @@ void esw_vport_cleanup_ingress_rules(struct
> mlx5_eswitch *esw,
>
> vport->ingress.drop_rule = NULL;
> vport->ingress.allow_rule = NULL;
> +
> + esw_vport_del_ingress_acl_modify_metadata(esw, vport);
> }
>
> void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw, diff --git
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> index 8b9f2cf58e91..4417a195832e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> @@ -68,6 +68,8 @@ struct vport_ingress {
> struct mlx5_flow_group *allow_spoofchk_only_grp;
> struct mlx5_flow_group *allow_untagged_only_grp;
> struct mlx5_flow_group *drop_grp;
> + int modify_metadata_id;
> + struct mlx5_flow_handle *modify_metadata_rule;
> struct mlx5_flow_handle *allow_rule;
> struct mlx5_flow_handle *drop_rule;
> struct mlx5_fc *drop_counter;
> @@ -196,6 +198,10 @@ struct mlx5_esw_functions {
> u16 num_vfs;
> };
>
> +enum {
> + MLX5_ESWITCH_VPORT_MATCH_METADATA = BIT(0), };
> +
> struct mlx5_eswitch {
> struct mlx5_core_dev *dev;
> struct mlx5_nb nb;
> @@ -203,6 +209,7 @@ struct mlx5_eswitch {
> struct hlist_head mc_table[MLX5_L2_ADDR_HASH_SIZE];
> struct workqueue_struct *work_queue;
> struct mlx5_vport *vports;
> + u32 flags;
> int total_vports;
> int enabled_vports;
> /* Synchronize between vport change events @@ -240,6 +247,8 @@
> void esw_vport_disable_egress_acl(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport);
> void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport);
> +void esw_vport_del_ingress_acl_modify_metadata(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport);
>
> /* E-Switch API */
> int mlx5_eswitch_init(struct mlx5_core_dev *dev); diff --git
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 17abb98b48af..871ae44dc132 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -1555,32 +1555,16 @@ static void esw_offloads_devcom_cleanup(struct
> mlx5_eswitch *esw) static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> struct mlx5_vport *vport)
> {
> - struct mlx5_core_dev *dev = esw->dev;
> struct mlx5_flow_act flow_act = {0};
> struct mlx5_flow_spec *spec;
> int err = 0;
>
> /* For prio tag mode, there is only 1 FTEs:
> - * 1) Untagged packets - push prio tag VLAN, allow
> + * 1) Untagged packets - push prio tag VLAN and modify metadata if
> + * required, allow
> * Unmatched traffic is allowed by default
> */
>
> - if (!MLX5_CAP_ESW_INGRESS_ACL(dev, ft_support))
> - return -EOPNOTSUPP;
> -
> - esw_vport_cleanup_ingress_rules(esw, vport);
> -
> - err = esw_vport_enable_ingress_acl(esw, vport);
> - if (err) {
> - mlx5_core_warn(esw->dev,
> - "failed to enable prio tag ingress acl (%d) on
> vport[%d]\n",
> - err, vport->vport);
> - return err;
> - }
> -
> - esw_debug(esw->dev,
> - "vport[%d] configure ingress rules\n", vport->vport);
> -
> spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
> if (!spec) {
> err = -ENOMEM;
> @@ -1596,6 +1580,12 @@ static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> flow_act.vlan[0].ethtype = ETH_P_8021Q;
> flow_act.vlan[0].vid = 0;
> flow_act.vlan[0].prio = 0;
> +
> + if (vport->ingress.modify_metadata_rule) {
> + flow_act.action |=
> MLX5_FLOW_CONTEXT_ACTION_MOD_HDR;
> + flow_act.modify_id = vport->ingress.modify_metadata_id;
> + }
> +
> vport->ingress.allow_rule =
> mlx5_add_flow_rules(vport->ingress.acl, spec,
> &flow_act, NULL, 0);
> @@ -1616,6 +1606,59 @@ static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> return err;
> }
>
> +static int esw_vport_add_ingress_acl_modify_metadata(struct mlx5_eswitch
> *esw,
> + struct mlx5_vport *vport)
> +{
> + u8 action[MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto)] =
> {};
> + struct mlx5_flow_act flow_act = {};
> + struct mlx5_flow_spec spec = {};
> + int err = 0;
> +
> + MLX5_SET(set_action_in, action, action_type,
> MLX5_ACTION_TYPE_SET);
> + MLX5_SET(set_action_in, action, field,
> MLX5_ACTION_IN_FIELD_METADATA_REG_C_0);
> + MLX5_SET(set_action_in, action, data,
> + mlx5_eswitch_get_vport_metadata_for_match(esw, vport-
> >vport));
> +
> + err = mlx5_modify_header_alloc(esw->dev,
> MLX5_FLOW_NAMESPACE_ESW_INGRESS,
> + 1, action, &vport-
> >ingress.modify_metadata_id);
> +
Please remove the empty line.
> + if (err) {
> + esw_warn(esw->dev,
> + "failed to alloc modify header for vport %d ingress acl
> (%d)\n",
> + vport->vport, err);
> + return err;
> + }
> +
> + flow_act.action = MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
> MLX5_FLOW_CONTEXT_ACTION_ALLOW;
> + flow_act.modify_id = vport->ingress.modify_metadata_id;
> + vport->ingress.modify_metadata_rule = mlx5_add_flow_rules(vport-
> >ingress.acl,
> + &spec,
> &flow_act, NULL, 0);
> + if (IS_ERR(vport->ingress.modify_metadata_rule)) {
> + err = PTR_ERR(vport->ingress.modify_metadata_rule);
> + esw_warn(esw->dev,
> + "failed to add setting metadata rule for vport %d
> ingress acl, err(%d)\n",
> + vport->vport, err);
> + vport->ingress.modify_metadata_rule = NULL;
> + goto out;
> + }
> +
> +out:
> + if (err)
> + mlx5_modify_header_dealloc(esw->dev, vport-
> >ingress.modify_metadata_id);
> + return err;
> +}
> +
> +void esw_vport_del_ingress_acl_modify_metadata(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport)
> +{
> + if (vport->ingress.modify_metadata_rule) {
> + mlx5_del_flow_rules(vport->ingress.modify_metadata_rule);
> + mlx5_modify_header_dealloc(esw->dev,
> +vport->ingress.modify_metadata_id);
> +
> + vport->ingress.modify_metadata_rule = NULL;
> + }
> +}
> +
> static int esw_vport_egress_prio_tag_config(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport)
> {
> @@ -1623,6 +1666,9 @@ static int esw_vport_egress_prio_tag_config(struct
> mlx5_eswitch *esw,
> struct mlx5_flow_spec *spec;
> int err = 0;
>
> + if (!MLX5_CAP_GEN(esw->dev, prio_tag_required))
> + return 0;
> +
> /* For prio tag mode, there is only 1 FTEs:
> * 1) prio tag packets - pop the prio tag VLAN, allow
> * Unmatched traffic is allowed by default @@ -1676,27 +1722,77 @@
> static int esw_vport_egress_prio_tag_config(struct mlx5_eswitch *esw,
> return err;
> }
>
> -static int esw_prio_tag_acls_config(struct mlx5_eswitch *esw, int nvports)
> +static int esw_vport_ingress_common_config(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport)
> {
> - struct mlx5_vport *vport = NULL;
> - int i, j;
> int err;
>
> - mlx5_esw_for_each_vf_vport(esw, i, vport, nvports) {
> + if (!mlx5_eswitch_vport_match_metadata_enabled(esw) &&
> + !MLX5_CAP_GEN(esw->dev, prio_tag_required))
> + return 0;
> +
> + esw_vport_cleanup_ingress_rules(esw, vport);
> +
> + err = esw_vport_enable_ingress_acl(esw, vport);
> + if (err) {
> + esw_warn(esw->dev,
> + "failed to enable ingress acl (%d) on vport[%d]\n",
> + err, vport->vport);
> + return err;
> + }
> +
> + esw_debug(esw->dev,
> + "vport[%d] configure ingress rules\n", vport->vport);
> +
> + if (mlx5_eswitch_vport_match_metadata_enabled(esw)) {
> + err = esw_vport_add_ingress_acl_modify_metadata(esw,
> vport);
> + if (err)
> + goto out;
> + }
> +
> + if (MLX5_CAP_GEN(esw->dev, prio_tag_required) &&
> + (vport->vport >= MLX5_VPORT_FIRST_VF &&
> + vport->vport <= esw->dev->priv.sriov.num_vfs)) {
> err = esw_vport_ingress_prio_tag_config(esw, vport);
> if (err)
> - goto err_ingress;
> - err = esw_vport_egress_prio_tag_config(esw, vport);
> + goto out;
> + }
> +
> +out:
> + if (err)
> + esw_vport_disable_ingress_acl(esw, vport);
> + return err;
> +}
> +
> +static int esw_create_offloads_acl_tables(struct mlx5_eswitch *esw) {
> + struct mlx5_vport *vport;
> + int i, j;
> + int err;
> +
> + mlx5_esw_for_all_vports(esw, i, vport) {
> + err = esw_vport_ingress_common_config(esw, vport);
> if (err)
> - goto err_egress;
> + goto err_ingress;
> +
> + if (vport->vport >= MLX5_VPORT_FIRST_VF &&
> + vport->vport <= esw->dev->priv.sriov.num_vfs) {
> + err = esw_vport_egress_prio_tag_config(esw, vport);
> + if (err)
> + goto err_egress;
> + }
> }
>
> + if (mlx5_eswitch_vport_match_metadata_enabled(esw))
> + esw_info(esw->dev, "Use metadata reg_c as source vport to
> match\n");
> +
> return 0;
>
> err_egress:
> esw_vport_disable_ingress_acl(esw, vport);
> err_ingress:
> - mlx5_esw_for_each_vf_vport_reverse(esw, j, vport, i - 1) {
> + for (j = MLX5_VPORT_PF; j < i; j++) {
> + vport = &esw->vports[j];
> esw_vport_disable_egress_acl(esw, vport);
> esw_vport_disable_ingress_acl(esw, vport);
> }
> @@ -1704,15 +1800,17 @@ static int esw_prio_tag_acls_config(struct
> mlx5_eswitch *esw, int nvports)
> return err;
> }
>
> -static void esw_prio_tag_acls_cleanup(struct mlx5_eswitch *esw)
> +static void esw_destroy_offloads_acl_tables(struct mlx5_eswitch *esw)
> {
> struct mlx5_vport *vport;
> int i;
>
> - mlx5_esw_for_each_vf_vport(esw, i, vport, esw->nvports) {
> + mlx5_esw_for_all_vports(esw, i, vport) {
> esw_vport_disable_egress_acl(esw, vport);
> esw_vport_disable_ingress_acl(esw, vport);
> }
> +
> + esw->flags &= ~MLX5_ESWITCH_VPORT_MATCH_METADATA;
> }
>
> static int esw_offloads_steering_init(struct mlx5_eswitch *esw, int nvports)
> @@ -1722,15 +1820,13 @@ static int esw_offloads_steering_init(struct
> mlx5_eswitch *esw, int nvports)
> memset(&esw->fdb_table.offloads, 0, sizeof(struct offloads_fdb));
> mutex_init(&esw->fdb_table.offloads.fdb_prio_lock);
>
> - if (MLX5_CAP_GEN(esw->dev, prio_tag_required)) {
> - err = esw_prio_tag_acls_config(esw, nvports);
> - if (err)
> - return err;
> - }
> + err = esw_create_offloads_acl_tables(esw);
> + if (err)
> + return err;
>
> err = esw_create_offloads_fdb_tables(esw, nvports);
> if (err)
> - return err;
> + goto create_fdb_err;
>
> err = esw_create_offloads_table(esw, nvports);
> if (err)
> @@ -1748,6 +1844,9 @@ static int esw_offloads_steering_init(struct
> mlx5_eswitch *esw, int nvports)
> create_ft_err:
> esw_destroy_offloads_fdb_tables(esw);
>
> +create_fdb_err:
> + esw_destroy_offloads_acl_tables(esw);
> +
> return err;
> }
>
> @@ -1756,8 +1855,7 @@ static void esw_offloads_steering_cleanup(struct
> mlx5_eswitch *esw)
> esw_destroy_vport_rx_group(esw);
> esw_destroy_offloads_table(esw);
> esw_destroy_offloads_fdb_tables(esw);
> - if (MLX5_CAP_GEN(esw->dev, prio_tag_required))
> - esw_prio_tag_acls_cleanup(esw);
> + esw_destroy_offloads_acl_tables(esw);
> }
>
> static void esw_functions_changed_event_handler(struct work_struct *work)
> @@ -2290,3 +2388,16 @@ struct mlx5_eswitch_rep
> *mlx5_eswitch_vport_rep(struct mlx5_eswitch *esw,
> return mlx5_eswitch_get_rep(esw, vport); }
> EXPORT_SYMBOL(mlx5_eswitch_vport_rep);
> +
> +u32 mlx5_eswitch_vport_match_metadata_enabled(struct mlx5_eswitch
> *esw)
> +{
> + return esw->flags & MLX5_ESWITCH_VPORT_MATCH_METADATA;
> +}
> +EXPORT_SYMBOL(mlx5_eswitch_vport_match_metadata_enabled);
> +
> +u32 mlx5_eswitch_get_vport_metadata_for_match(struct mlx5_eswitch *esw,
> + u16 vport)
> +{
> + return ((MLX5_CAP_GEN(esw->dev, vhca_id) & 0xffff) << 16) | vport; }
> +EXPORT_SYMBOL(mlx5_eswitch_get_vport_metadata_for_match);
> diff --git a/include/linux/mlx5/eswitch.h b/include/linux/mlx5/eswitch.h index
> 174eec0871d9..d729f5e4d70a 100644
> --- a/include/linux/mlx5/eswitch.h
> +++ b/include/linux/mlx5/eswitch.h
> @@ -64,6 +64,9 @@ struct mlx5_flow_handle *
> mlx5_eswitch_add_send_to_vport_rule(struct mlx5_eswitch *esw,
> int vport, u32 sqn);
>
> +u32 mlx5_eswitch_vport_match_metadata_enabled(struct mlx5_eswitch
> +*esw);
> +u32 mlx5_eswitch_get_vport_metadata_for_match(struct mlx5_eswitch *esw,
> +u16 vport);
> +
As you might be aware that vport_num and vport_index are not one to one map, we have few bugs in that area.
To avoid confusion to developers and users of the API, please name any new API as either vport_num or vport_index.
This makes it clear and explicit, specially global APIs like this in include directory.
> #ifdef CONFIG_MLX5_ESWITCH
> enum devlink_eswitch_encap_mode
> mlx5_eswitch_get_encap_mode(const struct mlx5_core_dev *dev);
> --
> 2.21.0
^ permalink raw reply
* RE: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use index of rep for vport to IB port mapping
From: Parav Pandit @ 2019-06-18 10:47 UTC (permalink / raw)
To: Leon Romanovsky, Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Bodong Wang,
Mark Bloch
In-Reply-To: <20190618104220.GH4690@mtr-leonro.mtl.com>
Hi Leon,
> -----Original Message-----
> From: Leon Romanovsky
> Sent: Tuesday, June 18, 2019 4:12 PM
> To: Saeed Mahameed <saeedm@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Bodong Wang
> <bodong@mellanox.com>; Parav Pandit <parav@mellanox.com>; Mark Bloch
> <markb@mellanox.com>
> Subject: Re: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use index of rep
> for vport to IB port mapping
>
> On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> > From: Bodong Wang <bodong@mellanox.com>
> >
> > In the single IB device mode, the mapping between vport number and rep
> > relies on a counter. However for dynamic vport allocation, it is
> > desired to keep consistent map of eswitch vport and IB port.
> >
> > Hence, simplify code to remove the free running counter and instead
> > use the available vport index during load/unload sequence from the
> > eswitch.
> >
> > Signed-off-by: Bodong Wang <bodong@mellanox.com>
> > Suggested-by: Parav Pandit <parav@mellanox.com>
> > Reviewed-by: Parav Pandit <parav@mellanox.com>
>
> We are not adding multiple "*-by" for same user, please choose one.
>
Suggested-by was added by Bodong during our discussion. Later on when I did gerrit +1, RB tag got added.
> Thanks
^ permalink raw reply
* Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG
From: Florian Westphal @ 2019-06-18 10:45 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, wenxu, netfilter-devel, netdev
In-Reply-To: <20190618100423.tirukx3ro2fl4khs@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Jun 18, 2019 at 11:46:13AM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> [...]
> > > Could you describe this problem a bit more? Small example rule plus
> > > scenario.
> >
> > It was what wenxu reported originally:
> >
> > nft add rule bridge filter forward ip protocol counter ..
> >
> > The rule only matches if the ip packet is contained in an ethernet frame
> > without vlan tag -- and thats neither expected nor desirable.
> >
> > This rule works when using 'meta protocol ip' as dependency instead
> > of ether type ip (what we do now), because VLAN stripping will fix/alter
> > skb->protocol to the inner type when the VLAN tag gets removes.
> >
> > It will still fail in case there are several VLAN tags, so we might
> > need another meta expression that can figure out the l3 protocol type.
>
> How would that new meta expression would look like?
I thought about extending nft_exthdr.c for L2, i.e. take
ether->type, and then advance to next vlan header (if vlan type)
until it either reaches skb network offset or an unknown type
(which would then be considered the last/topmost one and the one
carrying the l3 protocol number).
^ permalink raw reply
* Re: [PATCH mlx5-next 14/15] {IB, net}/mlx5: E-Switch, Use index of rep for vport to IB port mapping
From: Leon Romanovsky @ 2019-06-18 10:42 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Bodong Wang,
Parav Pandit, Mark Bloch
In-Reply-To: <20190617192247.25107-15-saeedm@mellanox.com>
On Mon, Jun 17, 2019 at 07:23:37PM +0000, Saeed Mahameed wrote:
> From: Bodong Wang <bodong@mellanox.com>
>
> In the single IB device mode, the mapping between vport number and
> rep relies on a counter. However for dynamic vport allocation, it is
> desired to keep consistent map of eswitch vport and IB port.
>
> Hence, simplify code to remove the free running counter and instead
> use the available vport index during load/unload sequence from the
> eswitch.
>
> Signed-off-by: Bodong Wang <bodong@mellanox.com>
> Suggested-by: Parav Pandit <parav@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
We are not adding multiple "*-by" for same user, please choose one.
Thanks
^ permalink raw reply
* Re: [PATCH mlx5-next 15/15] RDMA/mlx5: Cleanup rep when doing unload
From: Leon Romanovsky @ 2019-06-18 10:38 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Bodong Wang,
Mark Bloch, Parav Pandit
In-Reply-To: <20190617192247.25107-16-saeedm@mellanox.com>
On Mon, Jun 17, 2019 at 07:23:39PM +0000, Saeed Mahameed wrote:
> From: Bodong Wang <bodong@mellanox.com>
>
> When an IB rep is loaded, netdev for the same vport is saved for later
> reference. However, it's not cleaned up when doing unload. For ECPF,
> kernel crashes when driver is referring to the already removed netdev.
>
> Following steps lead to a shown call trace:
> 1. Create n VFs from host PF
> 2. Distroy the VFs
> 3. Run "rdma link" from ARM
>
> Call trace:
> mlx5_ib_get_netdev+0x9c/0xe8 [mlx5_ib]
> mlx5_query_port_roce+0x268/0x558 [mlx5_ib]
> mlx5_ib_rep_query_port+0x14/0x34 [mlx5_ib]
> ib_query_port+0x9c/0xfc [ib_core]
> fill_port_info+0x74/0x28c [ib_core]
> nldev_port_get_doit+0x1a8/0x1e8 [ib_core]
> rdma_nl_rcv_msg+0x16c/0x1c0 [ib_core]
> rdma_nl_rcv+0xe8/0x144 [ib_core]
> netlink_unicast+0x184/0x214
> netlink_sendmsg+0x288/0x354
> sock_sendmsg+0x18/0x2c
> __sys_sendto+0xbc/0x138
> __arm64_sys_sendto+0x28/0x34
> el0_svc_common+0xb0/0x100
> el0_svc_handler+0x6c/0x84
> el0_svc+0x8/0xc
>
> Cleanup the rep and netdev reference when unloading IB rep.
>
> Fixes: 26628e2d58c9 ("RDMA/mlx5: Move to single device multiport ports in switchdev mode")
> Signed-off-by: Bodong Wang <bodong@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Reviewed-by: Parav Pandit <parav@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/infiniband/hw/mlx5/ib_rep.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
^ permalink raw reply
* Re: [PATCH mlx5-next 12/15] net/mlx5: E-Switch, Enable vport metadata matching if firmware supports it
From: Leon Romanovsky @ 2019-06-18 10:35 UTC (permalink / raw)
To: Parav Pandit
Cc: Saeed Mahameed, netdev@vger.kernel.org,
linux-rdma@vger.kernel.org, Jianbo Liu, Roi Dayan, Mark Bloch
In-Reply-To: <AM0PR05MB4866AA7738F3DE1D3CF47FFBD1EA0@AM0PR05MB4866.eurprd05.prod.outlook.com>
On Tue, Jun 18, 2019 at 10:24:49AM +0000, Parav Pandit wrote:
>
>
> > -----Original Message-----
> > From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> > owner@vger.kernel.org> On Behalf Of Saeed Mahameed
> > Sent: Tuesday, June 18, 2019 12:54 AM
> > To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> > <leonro@mellanox.com>
> > Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Jianbo Liu
> > <jianbol@mellanox.com>; Roi Dayan <roid@mellanox.com>; Mark Bloch
> > <markb@mellanox.com>
> > Subject: [PATCH mlx5-next 12/15] net/mlx5: E-Switch, Enable vport metadata
> > matching if firmware supports it
> >
> > From: Jianbo Liu <jianbol@mellanox.com>
> >
> > As the ingress ACL rules save vhca id and vport number to packet's metadata
> > REG_C_0, and the metadata matching for the rules in both fast path and slow
> > path are all added, enable this feature if supported.
> >
> > Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> > Reviewed-by: Roi Dayan <roid@mellanox.com>
> > Reviewed-by: Mark Bloch <markb@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> > ---
> > .../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 13 +++++++++++++
> > 1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > index 363517e29d4c..5124219a31de 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> > @@ -1906,12 +1906,25 @@ static int
> > esw_vport_ingress_common_config(struct mlx5_eswitch *esw,
> > return err;
> > }
> >
> > +static int esw_check_vport_match_metadata_supported(struct mlx5_eswitch
> > +*esw) {
> > + return (MLX5_CAP_ESW_FLOWTABLE(esw->dev,
> > fdb_to_vport_reg_c_id) &
> > + MLX5_FDB_TO_VPORT_REG_C_0) &&
> > + MLX5_CAP_ESW_FLOWTABLE(esw->dev, flow_source) &&
> > + MLX5_CAP_ESW(esw->dev, esw_uplink_ingress_acl) &&
> > + !mlx5_core_is_ecpf_esw_manager(esw->dev) &&
> > + !mlx5_ecpf_vport_exists(esw->dev);
> > +}
> > +
> struct mlx5_eswitch* should be const.
> return type should be bool.
It is also completely indigestible and should be break into peaces to
make it readable.
Thanks
^ permalink raw reply
* RE: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport number in VF vports and uplink ingress ACLs
From: Parav Pandit @ 2019-06-18 10:31 UTC (permalink / raw)
To: Saeed Mahameed, Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Eli Britstein, Roi Dayan, Mark Bloch
In-Reply-To: <20190617192247.25107-6-saeedm@mellanox.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Saeed Mahameed
> Sent: Tuesday, June 18, 2019 12:53 AM
> To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> <leonro@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Jianbo Liu
> <jianbol@mellanox.com>; Eli Britstein <elibr@mellanox.com>; Roi Dayan
> <roid@mellanox.com>; Mark Bloch <markb@mellanox.com>
> Subject: [PATCH mlx5-next 05/15] net/mlx5: E-Switch, Tag packet with vport
> number in VF vports and uplink ingress ACLs
>
> From: Jianbo Liu <jianbol@mellanox.com>
>
> When a dual-port VHCA sends a RoCE packet on its non-native port, and the
> packet arrives to its affiliated vport FDB, a mismatch might occur on the rules
> that match the packet source vport as it is not represented by single VHCA only
> in this case. So we change to match on metadata instead of source vport.
> To do that, a rule is created in all vports and uplink ingress ACLs, to save the
> source vport number and vhca id in the packet's metadata in order to match on
> it later.
> The metadata register used is the first of the 32-bit type C registers. It can be
> used for matching and header modify operations. The higher 16 bits of this
> register are for vhca id, and the lower 16 ones is for vport number.
> This change is not for dual-port RoCE only. If HW and FW allow, the vport
> metadata matching is enabled by default.
>
> Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> Reviewed-by: Eli Britstein <elibr@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/eswitch.c | 2 +
> .../net/ethernet/mellanox/mlx5/core/eswitch.h | 9 +
> .../mellanox/mlx5/core/eswitch_offloads.c | 183 ++++++++++++++----
> include/linux/mlx5/eswitch.h | 3 +
> 4 files changed, 161 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> index a42a23e505df..1235fd84ae3a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
> @@ -1168,6 +1168,8 @@ void esw_vport_cleanup_ingress_rules(struct
> mlx5_eswitch *esw,
>
> vport->ingress.drop_rule = NULL;
> vport->ingress.allow_rule = NULL;
> +
> + esw_vport_del_ingress_acl_modify_metadata(esw, vport);
> }
>
> void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw, diff --git
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> index 8b9f2cf58e91..4417a195832e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> @@ -68,6 +68,8 @@ struct vport_ingress {
> struct mlx5_flow_group *allow_spoofchk_only_grp;
> struct mlx5_flow_group *allow_untagged_only_grp;
> struct mlx5_flow_group *drop_grp;
> + int modify_metadata_id;
No need for random alignment. Just have one white space after int.
> + struct mlx5_flow_handle *modify_metadata_rule;
> struct mlx5_flow_handle *allow_rule;
> struct mlx5_flow_handle *drop_rule;
> struct mlx5_fc *drop_counter;
> @@ -196,6 +198,10 @@ struct mlx5_esw_functions {
> u16 num_vfs;
> };
>
> +enum {
> + MLX5_ESWITCH_VPORT_MATCH_METADATA = BIT(0), };
> +
> struct mlx5_eswitch {
> struct mlx5_core_dev *dev;
> struct mlx5_nb nb;
> @@ -203,6 +209,7 @@ struct mlx5_eswitch {
> struct hlist_head mc_table[MLX5_L2_ADDR_HASH_SIZE];
> struct workqueue_struct *work_queue;
> struct mlx5_vport *vports;
> + u32 flags;
Same as above, no need for extra aligment.
> int total_vports;
> int enabled_vports;
> /* Synchronize between vport change events @@ -240,6 +247,8 @@
> void esw_vport_disable_egress_acl(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport);
> void esw_vport_disable_ingress_acl(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport);
> +void esw_vport_del_ingress_acl_modify_metadata(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport);
>
> /* E-Switch API */
> int mlx5_eswitch_init(struct mlx5_core_dev *dev); diff --git
> a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 17abb98b48af..871ae44dc132 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -1555,32 +1555,16 @@ static void esw_offloads_devcom_cleanup(struct
> mlx5_eswitch *esw) static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> struct mlx5_vport *vport)
> {
> - struct mlx5_core_dev *dev = esw->dev;
> struct mlx5_flow_act flow_act = {0};
> struct mlx5_flow_spec *spec;
> int err = 0;
>
> /* For prio tag mode, there is only 1 FTEs:
> - * 1) Untagged packets - push prio tag VLAN, allow
> + * 1) Untagged packets - push prio tag VLAN and modify metadata if
> + * required, allow
> * Unmatched traffic is allowed by default
> */
>
> - if (!MLX5_CAP_ESW_INGRESS_ACL(dev, ft_support))
> - return -EOPNOTSUPP;
> -
> - esw_vport_cleanup_ingress_rules(esw, vport);
> -
> - err = esw_vport_enable_ingress_acl(esw, vport);
> - if (err) {
> - mlx5_core_warn(esw->dev,
> - "failed to enable prio tag ingress acl (%d) on
> vport[%d]\n",
> - err, vport->vport);
> - return err;
> - }
> -
> - esw_debug(esw->dev,
> - "vport[%d] configure ingress rules\n", vport->vport);
> -
> spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
> if (!spec) {
> err = -ENOMEM;
> @@ -1596,6 +1580,12 @@ static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> flow_act.vlan[0].ethtype = ETH_P_8021Q;
> flow_act.vlan[0].vid = 0;
> flow_act.vlan[0].prio = 0;
> +
> + if (vport->ingress.modify_metadata_rule) {
> + flow_act.action |=
> MLX5_FLOW_CONTEXT_ACTION_MOD_HDR;
> + flow_act.modify_id = vport->ingress.modify_metadata_id;
> + }
> +
> vport->ingress.allow_rule =
> mlx5_add_flow_rules(vport->ingress.acl, spec,
> &flow_act, NULL, 0);
> @@ -1616,6 +1606,59 @@ static int esw_vport_ingress_prio_tag_config(struct
> mlx5_eswitch *esw,
> return err;
> }
>
> +static int esw_vport_add_ingress_acl_modify_metadata(struct mlx5_eswitch
> *esw,
> + struct mlx5_vport *vport)
> +{
> + u8 action[MLX5_UN_SZ_BYTES(set_action_in_add_action_in_auto)] =
> {};
> + struct mlx5_flow_act flow_act = {};
> + struct mlx5_flow_spec spec = {};
> + int err = 0;
> +
> + MLX5_SET(set_action_in, action, action_type,
> MLX5_ACTION_TYPE_SET);
> + MLX5_SET(set_action_in, action, field,
> MLX5_ACTION_IN_FIELD_METADATA_REG_C_0);
> + MLX5_SET(set_action_in, action, data,
> + mlx5_eswitch_get_vport_metadata_for_match(esw, vport-
> >vport));
> +
> + err = mlx5_modify_header_alloc(esw->dev,
> MLX5_FLOW_NAMESPACE_ESW_INGRESS,
> + 1, action, &vport-
> >ingress.modify_metadata_id);
> +
> + if (err) {
> + esw_warn(esw->dev,
> + "failed to alloc modify header for vport %d ingress acl
> (%d)\n",
> + vport->vport, err);
> + return err;
> + }
> +
> + flow_act.action = MLX5_FLOW_CONTEXT_ACTION_MOD_HDR |
> MLX5_FLOW_CONTEXT_ACTION_ALLOW;
> + flow_act.modify_id = vport->ingress.modify_metadata_id;
> + vport->ingress.modify_metadata_rule = mlx5_add_flow_rules(vport-
> >ingress.acl,
> + &spec,
> &flow_act, NULL, 0);
> + if (IS_ERR(vport->ingress.modify_metadata_rule)) {
> + err = PTR_ERR(vport->ingress.modify_metadata_rule);
> + esw_warn(esw->dev,
> + "failed to add setting metadata rule for vport %d
> ingress acl, err(%d)\n",
> + vport->vport, err);
> + vport->ingress.modify_metadata_rule = NULL;
> + goto out;
> + }
> +
> +out:
> + if (err)
> + mlx5_modify_header_dealloc(esw->dev, vport-
> >ingress.modify_metadata_id);
> + return err;
> +}
> +
> +void esw_vport_del_ingress_acl_modify_metadata(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport)
> +{
> + if (vport->ingress.modify_metadata_rule) {
> + mlx5_del_flow_rules(vport->ingress.modify_metadata_rule);
> + mlx5_modify_header_dealloc(esw->dev,
> +vport->ingress.modify_metadata_id);
> +
> + vport->ingress.modify_metadata_rule = NULL;
> + }
> +}
> +
> static int esw_vport_egress_prio_tag_config(struct mlx5_eswitch *esw,
> struct mlx5_vport *vport)
> {
> @@ -1623,6 +1666,9 @@ static int esw_vport_egress_prio_tag_config(struct
> mlx5_eswitch *esw,
> struct mlx5_flow_spec *spec;
> int err = 0;
>
> + if (!MLX5_CAP_GEN(esw->dev, prio_tag_required))
> + return 0;
> +
> /* For prio tag mode, there is only 1 FTEs:
> * 1) prio tag packets - pop the prio tag VLAN, allow
> * Unmatched traffic is allowed by default @@ -1676,27 +1722,77 @@
> static int esw_vport_egress_prio_tag_config(struct mlx5_eswitch *esw,
> return err;
> }
>
> -static int esw_prio_tag_acls_config(struct mlx5_eswitch *esw, int nvports)
> +static int esw_vport_ingress_common_config(struct mlx5_eswitch *esw,
> + struct mlx5_vport *vport)
> {
> - struct mlx5_vport *vport = NULL;
> - int i, j;
> int err;
>
> - mlx5_esw_for_each_vf_vport(esw, i, vport, nvports) {
> + if (!mlx5_eswitch_vport_match_metadata_enabled(esw) &&
> + !MLX5_CAP_GEN(esw->dev, prio_tag_required))
> + return 0;
> +
> + esw_vport_cleanup_ingress_rules(esw, vport);
> +
> + err = esw_vport_enable_ingress_acl(esw, vport);
> + if (err) {
> + esw_warn(esw->dev,
> + "failed to enable ingress acl (%d) on vport[%d]\n",
> + err, vport->vport);
> + return err;
> + }
> +
> + esw_debug(esw->dev,
> + "vport[%d] configure ingress rules\n", vport->vport);
> +
> + if (mlx5_eswitch_vport_match_metadata_enabled(esw)) {
> + err = esw_vport_add_ingress_acl_modify_metadata(esw,
> vport);
> + if (err)
> + goto out;
> + }
> +
> + if (MLX5_CAP_GEN(esw->dev, prio_tag_required) &&
> + (vport->vport >= MLX5_VPORT_FIRST_VF &&
> + vport->vport <= esw->dev->priv.sriov.num_vfs)) {
> err = esw_vport_ingress_prio_tag_config(esw, vport);
> if (err)
> - goto err_ingress;
> - err = esw_vport_egress_prio_tag_config(esw, vport);
> + goto out;
> + }
> +
> +out:
> + if (err)
> + esw_vport_disable_ingress_acl(esw, vport);
> + return err;
> +}
> +
> +static int esw_create_offloads_acl_tables(struct mlx5_eswitch *esw) {
> + struct mlx5_vport *vport;
> + int i, j;
> + int err;
> +
> + mlx5_esw_for_all_vports(esw, i, vport) {
> + err = esw_vport_ingress_common_config(esw, vport);
> if (err)
> - goto err_egress;
> + goto err_ingress;
> +
> + if (vport->vport >= MLX5_VPORT_FIRST_VF &&
> + vport->vport <= esw->dev->priv.sriov.num_vfs) {
Add an helper API mlx5_esw_is_vport(const struct mlx5_esw *esw, const struct mlx5_vport *vport)
and use at two places in ingress and egress config.
> + err = esw_vport_egress_prio_tag_config(esw, vport);
> + if (err)
> + goto err_egress;
> + }
> }
>
> + if (mlx5_eswitch_vport_match_metadata_enabled(esw))
> + esw_info(esw->dev, "Use metadata reg_c as source vport to
> match\n");
> +
> return 0;
>
> err_egress:
> esw_vport_disable_ingress_acl(esw, vport);
> err_ingress:
> - mlx5_esw_for_each_vf_vport_reverse(esw, j, vport, i - 1) {
> + for (j = MLX5_VPORT_PF; j < i; j++) {
Keep the reverse order as before.
> + vport = &esw->vports[j];
> esw_vport_disable_egress_acl(esw, vport);
> esw_vport_disable_ingress_acl(esw, vport);
> }
> @@ -1704,15 +1800,17 @@ static int esw_prio_tag_acls_config(struct
> mlx5_eswitch *esw, int nvports)
> return err;
> }
>
> -static void esw_prio_tag_acls_cleanup(struct mlx5_eswitch *esw)
> +static void esw_destroy_offloads_acl_tables(struct mlx5_eswitch *esw)
> {
> struct mlx5_vport *vport;
> int i;
>
> - mlx5_esw_for_each_vf_vport(esw, i, vport, esw->nvports) {
> + mlx5_esw_for_all_vports(esw, i, vport) {
If you are changing this, please do in reverse order to keep it exact mirror of create/enable sequence.
> esw_vport_disable_egress_acl(esw, vport);
> esw_vport_disable_ingress_acl(esw, vport);
> }
> +
> + esw->flags &= ~MLX5_ESWITCH_VPORT_MATCH_METADATA;
> }
>
> static int esw_offloads_steering_init(struct mlx5_eswitch *esw, int nvports)
> @@ -1722,15 +1820,13 @@ static int esw_offloads_steering_init(struct
> mlx5_eswitch *esw, int nvports)
> memset(&esw->fdb_table.offloads, 0, sizeof(struct offloads_fdb));
> mutex_init(&esw->fdb_table.offloads.fdb_prio_lock);
>
> - if (MLX5_CAP_GEN(esw->dev, prio_tag_required)) {
> - err = esw_prio_tag_acls_config(esw, nvports);
> - if (err)
> - return err;
> - }
> + err = esw_create_offloads_acl_tables(esw);
> + if (err)
> + return err;
>
> err = esw_create_offloads_fdb_tables(esw, nvports);
> if (err)
> - return err;
> + goto create_fdb_err;
>
> err = esw_create_offloads_table(esw, nvports);
> if (err)
> @@ -1748,6 +1844,9 @@ static int esw_offloads_steering_init(struct
> mlx5_eswitch *esw, int nvports)
> create_ft_err:
> esw_destroy_offloads_fdb_tables(esw);
>
> +create_fdb_err:
> + esw_destroy_offloads_acl_tables(esw);
> +
> return err;
> }
>
> @@ -1756,8 +1855,7 @@ static void esw_offloads_steering_cleanup(struct
> mlx5_eswitch *esw)
> esw_destroy_vport_rx_group(esw);
> esw_destroy_offloads_table(esw);
> esw_destroy_offloads_fdb_tables(esw);
> - if (MLX5_CAP_GEN(esw->dev, prio_tag_required))
> - esw_prio_tag_acls_cleanup(esw);
> + esw_destroy_offloads_acl_tables(esw);
> }
>
> static void esw_functions_changed_event_handler(struct work_struct *work)
> @@ -2290,3 +2388,16 @@ struct mlx5_eswitch_rep
> *mlx5_eswitch_vport_rep(struct mlx5_eswitch *esw,
> return mlx5_eswitch_get_rep(esw, vport); }
> EXPORT_SYMBOL(mlx5_eswitch_vport_rep);
> +
> +u32 mlx5_eswitch_vport_match_metadata_enabled(struct mlx5_eswitch
> *esw)
> +{
> + return esw->flags & MLX5_ESWITCH_VPORT_MATCH_METADATA;
> +}
> +EXPORT_SYMBOL(mlx5_eswitch_vport_match_metadata_enabled);
> +
Return type should book and const *esw.
> +u32 mlx5_eswitch_get_vport_metadata_for_match(struct mlx5_eswitch *esw,
> + u16 vport)
> +{
> + return ((MLX5_CAP_GEN(esw->dev, vhca_id) & 0xffff) << 16) | vport; }
> +EXPORT_SYMBOL(mlx5_eswitch_get_vport_metadata_for_match);
This one too.
> diff --git a/include/linux/mlx5/eswitch.h b/include/linux/mlx5/eswitch.h index
> 174eec0871d9..d729f5e4d70a 100644
> --- a/include/linux/mlx5/eswitch.h
> +++ b/include/linux/mlx5/eswitch.h
> @@ -64,6 +64,9 @@ struct mlx5_flow_handle *
> mlx5_eswitch_add_send_to_vport_rule(struct mlx5_eswitch *esw,
> int vport, u32 sqn);
>
> +u32 mlx5_eswitch_vport_match_metadata_enabled(struct mlx5_eswitch
> +*esw);
> +u32 mlx5_eswitch_get_vport_metadata_for_match(struct mlx5_eswitch *esw,
> +u16 vport);
> +
> #ifdef CONFIG_MLX5_ESWITCH
> enum devlink_eswitch_encap_mode
> mlx5_eswitch_get_encap_mode(const struct mlx5_core_dev *dev);
> --
> 2.21.0
^ permalink raw reply
* RE: [PATCH mlx5-next 12/15] net/mlx5: E-Switch, Enable vport metadata matching if firmware supports it
From: Parav Pandit @ 2019-06-18 10:24 UTC (permalink / raw)
To: Saeed Mahameed, Saeed Mahameed, Leon Romanovsky
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Roi Dayan, Mark Bloch
In-Reply-To: <20190617192247.25107-13-saeedm@mellanox.com>
> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org <linux-rdma-
> owner@vger.kernel.org> On Behalf Of Saeed Mahameed
> Sent: Tuesday, June 18, 2019 12:54 AM
> To: Saeed Mahameed <saeedm@mellanox.com>; Leon Romanovsky
> <leonro@mellanox.com>
> Cc: netdev@vger.kernel.org; linux-rdma@vger.kernel.org; Jianbo Liu
> <jianbol@mellanox.com>; Roi Dayan <roid@mellanox.com>; Mark Bloch
> <markb@mellanox.com>
> Subject: [PATCH mlx5-next 12/15] net/mlx5: E-Switch, Enable vport metadata
> matching if firmware supports it
>
> From: Jianbo Liu <jianbol@mellanox.com>
>
> As the ingress ACL rules save vhca id and vport number to packet's metadata
> REG_C_0, and the metadata matching for the rules in both fast path and slow
> path are all added, enable this feature if supported.
>
> Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> .../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 363517e29d4c..5124219a31de 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -1906,12 +1906,25 @@ static int
> esw_vport_ingress_common_config(struct mlx5_eswitch *esw,
> return err;
> }
>
> +static int esw_check_vport_match_metadata_supported(struct mlx5_eswitch
> +*esw) {
> + return (MLX5_CAP_ESW_FLOWTABLE(esw->dev,
> fdb_to_vport_reg_c_id) &
> + MLX5_FDB_TO_VPORT_REG_C_0) &&
> + MLX5_CAP_ESW_FLOWTABLE(esw->dev, flow_source) &&
> + MLX5_CAP_ESW(esw->dev, esw_uplink_ingress_acl) &&
> + !mlx5_core_is_ecpf_esw_manager(esw->dev) &&
> + !mlx5_ecpf_vport_exists(esw->dev);
> +}
> +
struct mlx5_eswitch* should be const.
return type should be bool.
^ permalink raw reply
* Re: [PATCH mlx5-next 11/15] RDMA/mlx5: Add vport metadata matching for IB representors
From: Leon Romanovsky @ 2019-06-18 10:19 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org, Jianbo Liu,
Roi Dayan, Mark Bloch
In-Reply-To: <20190617192247.25107-12-saeedm@mellanox.com>
On Mon, Jun 17, 2019 at 07:23:30PM +0000, Saeed Mahameed wrote:
> From: Jianbo Liu <jianbol@mellanox.com>
>
> If vport metadata matching is enabled in eswitch, the rule created
> must be changed to match on the metadata, instead of source port.
>
> Signed-off-by: Jianbo Liu <jianbol@mellanox.com>
> Reviewed-by: Roi Dayan <roid@mellanox.com>
> Reviewed-by: Mark Bloch <markb@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/infiniband/hw/mlx5/ib_rep.c | 11 +++++++
> drivers/infiniband/hw/mlx5/ib_rep.h | 16 ++++++++++
> drivers/infiniband/hw/mlx5/main.c | 45 +++++++++++++++++++++++------
> 3 files changed, 63 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/ib_rep.c b/drivers/infiniband/hw/mlx5/ib_rep.c
> index 22e651cb5534..d4ed611de35d 100644
> --- a/drivers/infiniband/hw/mlx5/ib_rep.c
> +++ b/drivers/infiniband/hw/mlx5/ib_rep.c
> @@ -131,6 +131,17 @@ struct mlx5_eswitch_rep *mlx5_ib_vport_rep(struct mlx5_eswitch *esw, int vport)
> return mlx5_eswitch_vport_rep(esw, vport);
> }
>
> +u32 mlx5_ib_eswitch_vport_match_metadata_enabled(struct mlx5_eswitch *esw)
> +{
> + return mlx5_eswitch_vport_match_metadata_enabled(esw);
> +}
> +
> +u32 mlx5_ib_eswitch_get_vport_metadata_for_match(struct mlx5_eswitch *esw,
> + u16 vport)
> +{
> + return mlx5_eswitch_get_vport_metadata_for_match(esw, vport);
> +}
1. There is no need to introduce one line functions, call to that code directly.
2. It should be bool and not u32.
Thanks
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic
From: Jon Hunter @ 2019-06-18 10:18 UTC (permalink / raw)
To: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Russell King, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-tegra
In-Reply-To: <78EB27739596EE489E55E81C33FEC33A0B9C8DD9@DE02WEMBXB.internal.synopsys.com>
On 18/06/2019 10:46, Jose Abreu wrote:
> From: Jon Hunter <jonathanh@nvidia.com>
>
>> I am not certain but I don't believe so. We are using a static IP address
>> and mounting the root file-system via NFS when we see this ...
>
> Can you please add a call to napi_synchronize() before every
> napi_disable() calls, like this:
>
> if (queue < rx_queues_cnt) {
> napi_synchronize(&ch->rx_napi);
> napi_disable(&ch->rx_napi);
> }
>
> if (queue < tx_queues_cnt) {
> napi_synchronize(&ch->tx_napi);
> napi_disable(&ch->tx_napi);
> }
>
> [ I can send you a patch if you prefer ]
Yes I can try this and for completeness you mean ...
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 4ca46289a742..d4a12cb64d8e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -146,10 +146,15 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
for (queue = 0; queue < maxq; queue++) {
struct stmmac_channel *ch = &priv->channel[queue];
- if (queue < rx_queues_cnt)
+ if (queue < rx_queues_cnt) {
+ napi_synchronize(&ch->rx_napi);
napi_disable(&ch->rx_napi);
- if (queue < tx_queues_cnt)
+ }
+
+ if (queue < tx_queues_cnt) {
+ napi_synchronize(&ch->tx_napi);
napi_disable(&ch->tx_napi);
+ }
}
}
Cheers
Jon
--
nvpublic
^ permalink raw reply related
* Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG
From: Pablo Neira Ayuso @ 2019-06-18 10:04 UTC (permalink / raw)
To: Florian Westphal; +Cc: wenxu, netfilter-devel, netdev
In-Reply-To: <20190618094613.ztbwcclgsq54vkop@breakpoint.cc>
On Tue, Jun 18, 2019 at 11:46:13AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
[...]
> > Could you describe this problem a bit more? Small example rule plus
> > scenario.
>
> It was what wenxu reported originally:
>
> nft add rule bridge filter forward ip protocol counter ..
>
> The rule only matches if the ip packet is contained in an ethernet frame
> without vlan tag -- and thats neither expected nor desirable.
>
> This rule works when using 'meta protocol ip' as dependency instead
> of ether type ip (what we do now), because VLAN stripping will fix/alter
> skb->protocol to the inner type when the VLAN tag gets removes.
>
> It will still fail in case there are several VLAN tags, so we might
> need another meta expression that can figure out the l3 protocol type.
How would that new meta expression would look like?
^ permalink raw reply
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Ard Biesheuvel @ 2019-06-18 10:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Eric Biggers,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <CANn89i+X7YQ6DueDQAusA+1S5Kmo75OwzO+eYRZe_nR8=YWjuQ@mail.gmail.com>
On Tue, 18 Jun 2019 at 11:53, Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jun 18, 2019 at 2:41 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > On Tue, 18 Jun 2019 at 11:39, Eric Dumazet <edumazet@google.com> wrote:
> > >
> > > On Tue, Jun 18, 2019 at 2:32 AM Ard Biesheuvel
> > > <ard.biesheuvel@linaro.org> wrote:
> > > >
> > > > Some changes to the TCP fastopen code to make it more robust
> > > > against future changes in the choice of key/cookie size, etc.
> > > >
> > > > - Instead of keeping the SipHash key in an untyped u8[] buffer
> > > > and casting it to the right type upon use, use the correct
> > > > siphash_key_t type directly. This ensures that the key will
> > > > appear at the correct alignment if we ever change the way
> > > > these data structures are allocated. (Currently, they are
> > > > only allocated via kmalloc so they always appear at the
> > > > correct alignment)
> > > >
> > > > - Use DIV_ROUND_UP when sizing the u64[] array to hold the
> > > > cookie, so it is always of sufficient size, even when
> > > > TCP_FASTOPEN_COOKIE_MAX is no longer a multiple of 8.
> > > >
> > > > - Add a key length check to tcp_fastopen_reset_cipher(). No
> > > > callers exist currently that fail this check (they all pass
> > > > compile constant values that equal TCP_FASTOPEN_KEY_LENGTH),
> > > > but future changes might create problems, e.g., by leaving part
> > > > of the key uninitialized, or overflowing the key buffers.
> > > >
> > > > Note that none of these are functional changes wrt the current
> > > > state of the code.
> > > >
> > > ...
> > >
> > > > - memcpy(ctx->key[0], primary_key, len);
> > > > + if (unlikely(len != TCP_FASTOPEN_KEY_LENGTH)) {
> > > > + pr_err("TCP: TFO key length %u invalid\n", len);
> > > > + err = -EINVAL;
> > > > + goto out;
> > > > + }
> > >
> > >
> > > Why a pr_err() is there ?
> > >
> > > Can unpriv users flood the syslog ?
> >
> > They can if they could do so before: there was a call to
> > crypto_cipher_setkey() in the original pre-SipHash code which would
> > also result in a pr_err() on an invalid key length. That call got
> > removed along with the AES cipher handling, and this basically
> > reinstates it, as suggested by EricB.
>
> This tcp_fastopen_reset_cipher() function is internal to TCP stack, all callers
> always pass the correct length.
>
> We could add checks all over the place, and end up having a TCP stack
> full of defensive
> checks and 10,000 additional lines of code :/
>
> I would prefer not reinstating this.
Fair enough.
^ permalink raw reply
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Eric Dumazet @ 2019-06-18 9:53 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: netdev, Eric Biggers,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <CAKv+Gu894bEEzpKNDTaNiiNJTFoUTYQuFjBBm-ezdkrzW5fyNQ@mail.gmail.com>
On Tue, Jun 18, 2019 at 2:41 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 18 Jun 2019 at 11:39, Eric Dumazet <edumazet@google.com> wrote:
> >
> > On Tue, Jun 18, 2019 at 2:32 AM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > >
> > > Some changes to the TCP fastopen code to make it more robust
> > > against future changes in the choice of key/cookie size, etc.
> > >
> > > - Instead of keeping the SipHash key in an untyped u8[] buffer
> > > and casting it to the right type upon use, use the correct
> > > siphash_key_t type directly. This ensures that the key will
> > > appear at the correct alignment if we ever change the way
> > > these data structures are allocated. (Currently, they are
> > > only allocated via kmalloc so they always appear at the
> > > correct alignment)
> > >
> > > - Use DIV_ROUND_UP when sizing the u64[] array to hold the
> > > cookie, so it is always of sufficient size, even when
> > > TCP_FASTOPEN_COOKIE_MAX is no longer a multiple of 8.
> > >
> > > - Add a key length check to tcp_fastopen_reset_cipher(). No
> > > callers exist currently that fail this check (they all pass
> > > compile constant values that equal TCP_FASTOPEN_KEY_LENGTH),
> > > but future changes might create problems, e.g., by leaving part
> > > of the key uninitialized, or overflowing the key buffers.
> > >
> > > Note that none of these are functional changes wrt the current
> > > state of the code.
> > >
> > ...
> >
> > > - memcpy(ctx->key[0], primary_key, len);
> > > + if (unlikely(len != TCP_FASTOPEN_KEY_LENGTH)) {
> > > + pr_err("TCP: TFO key length %u invalid\n", len);
> > > + err = -EINVAL;
> > > + goto out;
> > > + }
> >
> >
> > Why a pr_err() is there ?
> >
> > Can unpriv users flood the syslog ?
>
> They can if they could do so before: there was a call to
> crypto_cipher_setkey() in the original pre-SipHash code which would
> also result in a pr_err() on an invalid key length. That call got
> removed along with the AES cipher handling, and this basically
> reinstates it, as suggested by EricB.
This tcp_fastopen_reset_cipher() function is internal to TCP stack, all callers
always pass the correct length.
We could add checks all over the place, and end up having a TCP stack
full of defensive
checks and 10,000 additional lines of code :/
I would prefer not reinstating this.
^ permalink raw reply
* RE: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic
From: Jose Abreu @ 2019-06-18 9:46 UTC (permalink / raw)
To: Jon Hunter, Jose Abreu, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Russell King, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-tegra
In-Reply-To: <26cfaeff-a310-3b79-5b57-fd9c93bd8929@nvidia.com>
From: Jon Hunter <jonathanh@nvidia.com>
> I am not certain but I don't believe so. We are using a static IP address
> and mounting the root file-system via NFS when we see this ...
Can you please add a call to napi_synchronize() before every
napi_disable() calls, like this:
if (queue < rx_queues_cnt) {
napi_synchronize(&ch->rx_napi);
napi_disable(&ch->rx_napi);
}
if (queue < tx_queues_cnt) {
napi_synchronize(&ch->tx_napi);
napi_disable(&ch->tx_napi);
}
[ I can send you a patch if you prefer ]
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH] netfilter: nft_paylaod: add base type NFT_PAYLOAD_LL_HEADER_NO_TAG
From: Florian Westphal @ 2019-06-18 9:46 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, wenxu, netfilter-devel, netdev
In-Reply-To: <20190618093508.3c5jjmmmuz3m26uj@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Tue, Jun 18, 2019 at 12:42:32AM +0200, Florian Westphal wrote:
> > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Subject: Change bridge l3 dependency to meta protocol
> > > >
> > > > This examines skb->protocol instead of ethernet header type, which
> > > > might be different when vlan is involved.
> > > >
> > > > + if (ctx->pctx.family == NFPROTO_BRIDGE && desc == &proto_eth) {
> > > > + if (expr->payload.desc == &proto_ip ||
> > > > + expr->payload.desc == &proto_ip6)
> > > > + desc = &proto_metaeth;
> > > > + }i
> > >
> > > Is this sufficient to restrict the matching? Is this still buggy from
> > > ingress?
> >
> > This is what netdev family uses as well (skb->protocol i mean).
> > I'm not sure it will work for output however (haven't checked).
>
> You mean for locally generated traffic?
Yes.
> > > I wonder if an explicit NFT_PAYLOAD_CHECK_VLAN flag would be useful in
> > > the kernel, if so we could rename NFTA_PAYLOAD_CSUM_FLAGS to
> > > NFTA_PAYLOAD_FLAGS and place it there. Just an idea.
> >
> > What would NFT_PAYLOAD_CHECK_VLAN do?
>
> Similar to the checksum approach, it provides a hint to the kernel to
> say that "I want to look at the vlan header" from the link layer.
I see. Its a bit of a furhter problem because tags can be nested,
so we would have to provide a more dynamic approach, similar to tunnel
matching (vlan header 0 id 42 vlan header 1 id 23' etc).
> > What might be useful is an nft switch to turn off dependeny
> > insertion, this would also avoid the problem (if users restrict the
> > matching properly...).
>
> Hm. How does this toggle would look like?
nft --nodep add rule bridge filter input ip protocol icmp # broken, has false positives
nft --nodep add rule bridge filter input ip version 4 ip protocol icmp # might work
nft --nodep add rule bridge filter input meta protocol ip ip protocol icmp # might work too
Its kind of I-Know-What-I-Am-Doing switch ...
We can already do this with raw payload expressions but those aren't that user
friendly.
> > Another unresolved issue is presence of multiple vlan tags, so we might
> > have to add yet another meta key to retrieve the l3 protocol in use
> >
> > (the problem at hand was 'ip protocol icmp' not matching traffic inside
> > a vlan).
>
> Could you describe this problem a bit more? Small example rule plus
> scenario.
It was what wenxu reported originally:
nft add rule bridge filter forward ip protocol counter ..
The rule only matches if the ip packet is contained in an ethernet frame
without vlan tag -- and thats neither expected nor desirable.
This rule works when using 'meta protocol ip' as dependency instead
of ether type ip (what we do now), because VLAN stripping will fix/alter
skb->protocol to the inner type when the VLAN tag gets removes.
It will still fail in case there are several VLAN tags, so we might
need another meta expression that can figure out the l3 protocol type.
Does that make sense so far?
^ permalink raw reply
* Re: [PATCH net-next 3/3] net: stmmac: Convert to phylink and remove phylib logic
From: Jon Hunter @ 2019-06-18 9:42 UTC (permalink / raw)
To: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Cc: Joao Pinto, David S . Miller, Giuseppe Cavallaro,
Alexandre Torgue, Russell King, Andrew Lunn, Florian Fainelli,
Heiner Kallweit, linux-tegra
In-Reply-To: <78EB27739596EE489E55E81C33FEC33A0B9C8D6E@DE02WEMBXB.internal.synopsys.com>
On 18/06/2019 10:35, Jose Abreu wrote:
> From: Jon Hunter <jonathanh@nvidia.com>
>
>> I am seeing a boot regression on -next for some of our boards that have
>> a synopsys ethernet controller that uses the dwmac-dwc-qos-ethernet
>> driver. Git bisect is pointing to this commit, but unfortunately this
>> cannot be cleanly reverted on top of -next to confirm.
>
> Thanks for reporting. Looks like the timer is not setup when
> stmmac_tx_clean() is called. When do you see this stacktrace ? After
> ifdown ?
I am not certain but I don't believe so. We are using a static IP address
and mounting the root file-system via NFS when we see this ...
[ 10.607510] dwc-eth-dwmac 2490000.ethernet eth0: phy link up rgmii/1Gbps/Full
[ 10.607536] dwc-eth-dwmac 2490000.ethernet eth0: phylink_mac_config: mode=phy/rgmii/1Gbps/Full adv=00,00000000,00000000 pause=0f link=1 an=0
[ 10.608804] dwc-eth-dwmac 2490000.ethernet eth0: Link is Up - 1Gbps/Full - flow control rx/tx
[ 10.630979] IP-Config: Complete:
[ 10.639046] device=eth0, hwaddr=d2:e5:1c:57:26:4b, ipaddr=192.168.99.2, mask=255.255.255.0, gw=192.168.99.1
[ 10.649201] host=192.168.99.2, domain=, nis-domain=(none)
[ 10.655022] bootserver=192.168.0.1, rootserver=192.168.0.1, rootpath=
[ 10.677531] VDD_1V8_AP_PLL: disabling
[ 10.681194] VDD_RTC: disabling
[ 10.684246] VDDIO_SDMMC3_AP: disabling
[ 10.688132] VDD_HDMI_1V05: disabling
[ 10.691704] SD_CARD_SW_PWR: disabling
[ 10.695357] VDD_USB0: disabling
[ 10.698488] VDD_USB1: disabling
[ 10.701621] VDD_HDMI_5V0: disabling
[ 10.705100] ALSA device list:
[ 10.708063] No soundcards found.
[ 10.711914] Freeing unused kernel memory: 1472K
[ 10.727005] Run /init as init process
[ 10.784989] ------------[ cut here ]------------
[ 10.789597] kernel BUG at /home/jonathanh/workdir/tegra/mlt-linux_next/kernel/kernel/time/timer.c:952!
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Ard Biesheuvel @ 2019-06-18 9:41 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev, Eric Biggers,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu,
David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <CANn89iJuTq36KMf1madQH08g6K0a-Uj-PDH80ao9zuEw+WNcZg@mail.gmail.com>
On Tue, 18 Jun 2019 at 11:39, Eric Dumazet <edumazet@google.com> wrote:
>
> On Tue, Jun 18, 2019 at 2:32 AM Ard Biesheuvel
> <ard.biesheuvel@linaro.org> wrote:
> >
> > Some changes to the TCP fastopen code to make it more robust
> > against future changes in the choice of key/cookie size, etc.
> >
> > - Instead of keeping the SipHash key in an untyped u8[] buffer
> > and casting it to the right type upon use, use the correct
> > siphash_key_t type directly. This ensures that the key will
> > appear at the correct alignment if we ever change the way
> > these data structures are allocated. (Currently, they are
> > only allocated via kmalloc so they always appear at the
> > correct alignment)
> >
> > - Use DIV_ROUND_UP when sizing the u64[] array to hold the
> > cookie, so it is always of sufficient size, even when
> > TCP_FASTOPEN_COOKIE_MAX is no longer a multiple of 8.
> >
> > - Add a key length check to tcp_fastopen_reset_cipher(). No
> > callers exist currently that fail this check (they all pass
> > compile constant values that equal TCP_FASTOPEN_KEY_LENGTH),
> > but future changes might create problems, e.g., by leaving part
> > of the key uninitialized, or overflowing the key buffers.
> >
> > Note that none of these are functional changes wrt the current
> > state of the code.
> >
> ...
>
> > - memcpy(ctx->key[0], primary_key, len);
> > + if (unlikely(len != TCP_FASTOPEN_KEY_LENGTH)) {
> > + pr_err("TCP: TFO key length %u invalid\n", len);
> > + err = -EINVAL;
> > + goto out;
> > + }
>
>
> Why a pr_err() is there ?
>
> Can unpriv users flood the syslog ?
They can if they could do so before: there was a call to
crypto_cipher_setkey() in the original pre-SipHash code which would
also result in a pr_err() on an invalid key length. That call got
removed along with the AES cipher handling, and this basically
reinstates it, as suggested by EricB.
^ permalink raw reply
* Re: [PATCH 1/2] net: fastopen: make key handling more robust against future changes
From: Eric Dumazet @ 2019-06-18 9:39 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: netdev, Eric Biggers, linux-crypto, Herbert Xu, David Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Jason Baron,
Christoph Paasch, David Laight, Yuchung Cheng
In-Reply-To: <20190618093207.13436-2-ard.biesheuvel@linaro.org>
On Tue, Jun 18, 2019 at 2:32 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> Some changes to the TCP fastopen code to make it more robust
> against future changes in the choice of key/cookie size, etc.
>
> - Instead of keeping the SipHash key in an untyped u8[] buffer
> and casting it to the right type upon use, use the correct
> siphash_key_t type directly. This ensures that the key will
> appear at the correct alignment if we ever change the way
> these data structures are allocated. (Currently, they are
> only allocated via kmalloc so they always appear at the
> correct alignment)
>
> - Use DIV_ROUND_UP when sizing the u64[] array to hold the
> cookie, so it is always of sufficient size, even when
> TCP_FASTOPEN_COOKIE_MAX is no longer a multiple of 8.
>
> - Add a key length check to tcp_fastopen_reset_cipher(). No
> callers exist currently that fail this check (they all pass
> compile constant values that equal TCP_FASTOPEN_KEY_LENGTH),
> but future changes might create problems, e.g., by leaving part
> of the key uninitialized, or overflowing the key buffers.
>
> Note that none of these are functional changes wrt the current
> state of the code.
>
...
> - memcpy(ctx->key[0], primary_key, len);
> + if (unlikely(len != TCP_FASTOPEN_KEY_LENGTH)) {
> + pr_err("TCP: TFO key length %u invalid\n", len);
> + err = -EINVAL;
> + goto out;
> + }
Why a pr_err() is there ?
Can unpriv users flood the syslog ?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox