| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250410143821.000002c0@gmail.com>
Date: Thu, 10 Apr 2025 14:38:21 +0800
From: Furong Xu <0x1207@...il.com>
To: Boon Khai Ng <boon.khai.ng@...era.com>
Cc: netdev@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
bpf@...r.kernel.org, Andrew Lunn <andrew+netdev@...n.ch>, "David S .
Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Maxime
Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
<alexandre.torgue@...s.st.com>, Russell King <linux@...linux.org.uk>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
<daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, John
Fastabend <john.fastabend@...il.com>, Matthew Gerlach
<matthew.gerlach@...era.com>, Tien Sung Ang <tien.sung.ang@...era.com>, Mun
Yew Tham <mun.yew.tham@...era.com>, G Thomas Rohan
<rohan.g.thomas@...era.com>
Subject: Re: [PATCH net-next v3 1/2] net: stmmac: Refactor VLAN
implementation
On Tue, 8 Apr 2025 16:13:53 +0800, Boon Khai Ng <boon.khai.ng@...era.com> wrote:
> Refactor VLAN implementation by moving common code for DWMAC4 and
> DWXGMAC IPs into a separate VLAN module. VLAN implementation for
> DWMAC4 and DWXGMAC differs only for CSR base address, the descriptor
> for the VLAN ID and VLAN VALID bit field.
>
> Signed-off-by: Boon Khai Ng <boon.khai.ng@...era.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@...era.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/Makefile | 2 +-
> drivers/net/ethernet/stmicro/stmmac/common.h | 1 +
> drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 40 ---
> .../net/ethernet/stmicro/stmmac/dwmac4_core.c | 295 +-----------------
> .../net/ethernet/stmicro/stmmac/dwxgmac2.h | 13 -
> .../ethernet/stmicro/stmmac/dwxgmac2_core.c | 87 ------
> drivers/net/ethernet/stmicro/stmmac/hwif.c | 8 +
> drivers/net/ethernet/stmicro/stmmac/hwif.h | 61 ++--
> .../net/ethernet/stmicro/stmmac/stmmac_vlan.c | 294 +++++++++++++++++
> .../net/ethernet/stmicro/stmmac/stmmac_vlan.h | 63 ++++
> 10 files changed, 401 insertions(+), 463 deletions(-)
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.c
> create mode 100644 drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.h
>
[...]
> diff --git a/drivers/net/ethernet/stmicro/stmmac/hwif.c b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> index 31bdbab9a46c..0a57c5e7497d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/hwif.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/hwif.c
> @@ -9,6 +9,7 @@
> #include "stmmac_fpe.h"
> #include "stmmac_ptp.h"
> #include "stmmac_est.h"
> +#include "stmmac_vlan.h"
> #include "dwmac4_descs.h"
> #include "dwxgmac2.h"
>
> @@ -120,6 +121,7 @@ static const struct stmmac_hwif_entry {
> const void *tc;
> const void *mmc;
> const void *est;
> + const void *vlan;
> int (*setup)(struct stmmac_priv *priv);
> int (*quirks)(struct stmmac_priv *priv);
> } stmmac_hw[] = {
> @@ -197,6 +199,7 @@ static const struct stmmac_hwif_entry {
> .desc = &dwmac4_desc_ops,
> .dma = &dwmac4_dma_ops,
> .mac = &dwmac410_ops,
> + .vlan = &dwmac_vlan_ops,
Rename dwmac_vlan_ops to dwmac4_vlan_ops will be better,
just like dwmac4_desc_ops/dwmac4_dma_ops
[...]
> +const struct stmmac_vlan_ops dwmac_vlan_ops = {
> + .update_vlan_hash = vlan_update_hash,
> + .enable_vlan = vlan_enable,
> + .add_hw_vlan_rx_fltr = vlan_add_hw_rx_fltr,
> + .del_hw_vlan_rx_fltr = vlan_del_hw_rx_fltr,
> + .restore_hw_vlan_rx_fltr = vlan_restore_hw_rx_fltr,
> + .rx_hw_vlan = vlan_rx_hw,
> + .set_hw_vlan_mode = vlan_set_hw_mode,
> +};
> +
> +const struct stmmac_vlan_ops dwxlgmac2_vlan_ops = {
> + .update_vlan_hash = vlan_update_hash,
> + .enable_vlan = vlan_enable,
> +};
dwxlgmac2_vlan_ops looks redundant here, another new struct contains
totally identical members.
stmmac_do_void_callback()/stmmac_do_callback() handles NULL function
pointers so good, we can leave the un-implemented functions as NULL.
Are you trying to avoid something undefined here?
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.h
> new file mode 100644
> index 000000000000..29e7be83161e
> --- /dev/null
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_vlan.h
> @@ -0,0 +1,63 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2025, Altera Corporation
> + * stmmac VLAN(802.1Q) handling
> + */
> +
> +#ifndef __STMMAC_VLAN_H__
> +#define __STMMAC_VLAN_H__
> +
> +#include <linux/bitfield.h>
> +
> +#define VLAN_TAG 0x00000050
> +#define VLAN_TAG_DATA 0x00000054
> +#define VLAN_HASH_TABLE 0x00000058
> +#define VLAN_INCL 0x00000060
> +
> +#define HW_FEATURE3 0x00000128
> +
> +/* MAC VLAN */
> +#define VLAN_EDVLP BIT(26)
> +#define VLAN_VTHM BIT(25)
> +#define VLAN_DOVLTC BIT(20)
> +#define VLAN_ESVL BIT(18)
> +#define VLAN_ETV BIT(16)
> +#define VLAN_VID GENMASK(15, 0)
> +#define VLAN_VLTI BIT(20)
> +#define VLAN_CSVL BIT(19)
> +#define VLAN_VLC GENMASK(17, 16)
> +#define VLAN_VLC_SHIFT 16
> +#define VLAN_VLHT GENMASK(15, 0)
> +
> +/* MAC VLAN Tag */
> +#define VLAN_TAG_VID GENMASK(15, 0)
> +#define VLAN_TAG_ETV BIT(16)
> +
> +/* MAC VLAN Tag Control */
> +#define VLAN_TAG_CTRL_OB BIT(0)
> +#define VLAN_TAG_CTRL_CT BIT(1)
> +#define VLAN_TAG_CTRL_OFS_MASK GENMASK(6, 2)
> +#define VLAN_TAG_CTRL_OFS_SHIFT 2
> +#define VLAN_TAG_CTRL_EVLS_MASK GENMASK(22, 21)
> +#define VLAN_TAG_CTRL_EVLS_SHIFT 21
> +#define VLAN_TAG_CTRL_EVLRXS BIT(24)
> +
> +#define VLAN_TAG_STRIP_NONE FIELD_PREP(VLAN_TAG_CTRL_EVLS_MASK, 0x0)
> +#define VLAN_TAG_STRIP_PASS FIELD_PREP(VLAN_TAG_CTRL_EVLS_MASK, 0x1)
> +#define VLAN_TAG_STRIP_FAIL FIELD_PREP(VLAN_TAG_CTRL_EVLS_MASK, 0x2)
> +#define VLAN_TAG_STRIP_ALL FIELD_PREP(VLAN_TAG_CTRL_EVLS_MASK, 0x3)
> +
> +/* MAC VLAN Tag Data/Filter */
> +#define VLAN_TAG_DATA_VID GENMASK(15, 0)
> +#define VLAN_TAG_DATA_VEN BIT(16)
> +#define VLAN_TAG_DATA_ETV BIT(17)
> +
> +/* MAC VLAN HW FEAT */
> +#define VLAN_HW_FEAT_NRVF GENMASK(2, 0)
> +
> +extern const struct stmmac_vlan_ops dwmac_vlan_ops;
> +extern const struct stmmac_vlan_ops dwxlgmac2_vlan_ops;
> +
> +u32 stmmac_get_num_vlan(void __iomem *ioaddr);
> +
> +#endif /* __STMMAC_VLAN_H__ */
It is a good practice to only keep inside the header those definitions
which are truly exported by stmmac_vlan.c towards external callers.
That means those #defines which are only used within stmmac_vlan.c
shouldn't be here, but inside stmmac_vlan.c file.
Powered by blists - more mailing lists