| 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: <90add896bd48f5ba80385df4d8a4e27c91f97e7d.camel@microchip.com>
Date: Tue, 29 Nov 2022 05:25:55 +0000
From: <Arun.Ramadoss@...rochip.com>
To: <olteanv@...il.com>, <UNGLinuxDriver@...rochip.com>,
<vivien.didelot@...il.com>, <andrew@...n.ch>,
<f.fainelli@...il.com>, <kuba@...nel.org>, <edumazet@...gle.com>,
<pabeni@...hat.com>, <o.rempel@...gutronix.de>,
<Woojung.Huh@...rochip.com>, <davem@...emloft.net>
CC: <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org>,
<kernel@...gutronix.de>
Subject: Re: [PATCH v1 21/26] net: dsa: microchip: ksz8_r_sta_mac_table(): do
not use error code for empty entries
On Mon, 2022-11-28 at 12:59 +0100, Oleksij Rempel wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> This is a preparation for the next patch, to make use of read/write
> errors.
>
> Signed-off-by: Oleksij Rempel <o.rempel@...gutronix.de>
> ---
> drivers/net/dsa/microchip/ksz8795.c | 94 +++++++++++++++++--------
> ----
> 1 file changed, 54 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> index 1c08103c9f50..b7487be91f67 100644
> --- a/drivers/net/dsa/microchip/ksz8795.c
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -457,7 +457,7 @@ static int ksz8_r_dyn_mac_table(struct ksz_device
> *dev, u16 addr, u8 *mac_addr,
> }
>
> static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
> - struct alu_struct *alu)
> + struct alu_struct *alu, bool *valid)
> {
> u32 data_hi, data_lo;
> const u8 *shifts;
> @@ -470,28 +470,32 @@ static int ksz8_r_sta_mac_table(struct
> ksz_device *dev, u16 addr,
> ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
> data_hi = data >> 32;
> data_lo = (u32)data;
> - if (data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> - masks[STATIC_MAC_TABLE_OVERRIDE])) {
> - alu->mac[5] = (u8)data_lo;
> - alu->mac[4] = (u8)(data_lo >> 8);
> - alu->mac[3] = (u8)(data_lo >> 16);
> - alu->mac[2] = (u8)(data_lo >> 24);
> - alu->mac[1] = (u8)data_hi;
> - alu->mac[0] = (u8)(data_hi >> 8);
> - alu->port_forward =
> - (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS])
> >>
> - shifts[STATIC_MAC_FWD_PORTS];
> - alu->is_override =
> - (data_hi & masks[STATIC_MAC_TABLE_OVERRIDE])
> ? 1 : 0;
> - data_hi >>= 1;
> - alu->is_static = true;
> - alu->is_use_fid =
> - (data_hi & masks[STATIC_MAC_TABLE_USE_FID]) ?
> 1 : 0;
> - alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> - shifts[STATIC_MAC_FID];
> +
> + if (!(data_hi & (masks[STATIC_MAC_TABLE_VALID] |
> + masks[STATIC_MAC_TABLE_OVERRIDE]))) {
> + *valid = false;
> return 0;
> }
> - return -ENXIO;
> +
> + alu->mac[5] = (u8)data_lo;
> + alu->mac[4] = (u8)(data_lo >> 8);
> + alu->mac[3] = (u8)(data_lo >> 16);
> + alu->mac[2] = (u8)(data_lo >> 24);
> + alu->mac[1] = (u8)data_hi;
> + alu->mac[0] = (u8)(data_hi >> 8);
u64_to_ether_addr macro can be used to store address into array.
> + alu->port_forward =
> + (data_hi & masks[STATIC_MAC_TABLE_FWD_PORTS]) >>
> + shifts[STATIC_MAC_FWD_PORTS];
> + alu->is_override = (data_hi &
> masks[STATIC_MAC_TABLE_OVERRIDE]) ? 1 : 0;
> + data_hi >>= 1;
> + alu->is_static = true;
> + alu->is_use_fid = (data_hi & masks[STATIC_MAC_TABLE_USE_FID])
> ? 1 : 0;
> + alu->fid = (data_hi & masks[STATIC_MAC_TABLE_FID]) >>
> + shifts[STATIC_MAC_FID];
Instead of masks and shifts, you consider using
FIELD_GET(STATIC_MAC_TABLE_FID, data_hi);
> +
> + *valid = true;
> +
> + return 0;
> }
>
> void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
> @@ -969,12 +973,13 @@ int ksz8_fdb_dump(struct ksz_device *dev, int
> port,
>
> for (i = 0; i < dev->info->num_statics; i++) {
> struct alu_struct alu;
> + bool valid;
>
> - ret = ksz8_r_sta_mac_table(dev, i, &alu);
> - if (ret == -ENXIO)
> - continue;
> + ret = ksz8_r_sta_mac_table(dev, i, &alu, &valid);
> if (ret)
> return ret;
> + if (!valid)
> + continue;
>
> if (!(alu.port_forward & BIT(port)))
> continue;
> @@ -1010,20 +1015,25 @@ static int ksz8_add_sta_mac(struct ksz_device
> *dev, int port,
> const unsigned char *addr, u16 vid)
> {
> struct alu_struct alu;
> - int index;
> + int index, ret;
> int empty = 0;
>
> alu.port_forward = 0;
> for (index = 0; index < dev->info->num_statics; index++) {
> - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> - /* Found one already in static MAC table. */
> - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> - alu.fid == vid)
> - break;
> - /* Remember the first empty entry. */
> - } else if (!empty) {
> - empty = index + 1;
> + bool valid;
> +
> + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> + if (ret)
> + return ret;
> + if (!valid) {
> + /* Remember the first empty entry. */
> + if (!empty)
> + empty = index + 1;
> + continue;
> }
> +
> + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> vid)
> + break;
> }
>
> /* no available entry */
> @@ -1053,15 +1063,19 @@ static int ksz8_del_sta_mac(struct ksz_device
> *dev, int port,
> const unsigned char *addr, u16 vid)
> {
> struct alu_struct alu;
> - int index;
> + int index, ret;
Variable declaration in separate line.
>
> for (index = 0; index < dev->info->num_statics; index++) {
> - if (!ksz8_r_sta_mac_table(dev, index, &alu)) {
> - /* Found one already in static MAC table. */
> - if (!memcmp(alu.mac, addr, ETH_ALEN) &&
> - alu.fid == vid)
> - break;
> - }
> + bool valid;
> +
> + ret = ksz8_r_sta_mac_table(dev, index, &alu, &valid);
> + if (ret)
> + return ret;
> + if (!valid)
> + continue;
> +
> + if (!memcmp(alu.mac, addr, ETH_ALEN) && alu.fid ==
> vid)
> + break;
> }
>
> /* no available entry */
> --
> 2.30.2
>
Powered by blists - more mailing lists