netdev - Re: [PATCH net-next v3 3/3] net: dsa: microchip: ksz8: add MTU configuration support

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 8 Nov 2022 11:21:16 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Oleksij Rempel <o.rempel@...gutronix.de>
Cc:     Woojung Huh <woojung.huh@...rochip.com>,
        UNGLinuxDriver@...rochip.com, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, kernel@...gutronix.de,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        Arun.Ramadoss@...rochip.com
Subject: Re: [PATCH net-next v3 3/3] net: dsa: microchip: ksz8: add MTU
 configuration support

On Tue, Nov 08, 2022 at 06:43:36AM +0100, Oleksij Rempel wrote:
> +static int ksz8863_change_mtu(struct ksz_device *dev, int port, int max_frame)

Don't pass "int port" if you're not going to use it.

> +{
> +	u8 ctrl2 = 0;
> +
> +	if (max_frame <= KSZ8_LEGAL_PACKET_SIZE)
> +		ctrl2 |= KSZ8863_LEGAL_PACKET_ENABLE;
> +	else if (max_frame > KSZ8863_NORMAL_PACKET_SIZE)
> +		ctrl2 |= KSZ8863_HUGE_PACKET_ENABLE;
> +
> +	return ksz_rmw8(dev, REG_SW_CTRL_2, KSZ8863_LEGAL_PACKET_ENABLE
> +			| KSZ8863_HUGE_PACKET_ENABLE, ctrl2);

Coding conventions are to not start a new line with an operator, but to
put it at the end of the previous line:

	return ksz_rmw8(dev, REG_SW_CTRL_2, KSZ8863_LEGAL_PACKET_ENABLE |
			KSZ8863_HUGE_PACKET_ENABLE, ctrl2);

> +}
> +
> +static int ksz8795_change_mtu(struct ksz_device *dev, int port, int max_frame)

Same.

> +{
> +	u8 ctrl1 = 0, ctrl2 = 0;
> +	int ret;
> +
> +	if (max_frame > KSZ8_LEGAL_PACKET_SIZE)
> +		ctrl2 |= SW_LEGAL_PACKET_DISABLE;
> +	else if (max_frame > KSZ8863_NORMAL_PACKET_SIZE)
> +		ctrl1 |= SW_HUGE_PACKET;
> +
> +	ret = ksz_rmw8(dev, REG_SW_CTRL_1, SW_HUGE_PACKET, ctrl1);
> +	if (ret)
> +		return ret;
> +
> +	return ksz_rmw8(dev, REG_SW_CTRL_2, SW_LEGAL_PACKET_DISABLE, ctrl2);
> +}
> +
> +int ksz8_change_mtu(struct ksz_device *dev, int port, int mtu)
> +{
> +	u16 frame_size, max_frame = 0;
> +	int i;
> +
> +	frame_size = mtu + VLAN_ETH_HLEN + ETH_FCS_LEN;
> +
> +	/* Cache the per-port MTU setting */
> +	dev->ports[port].max_frame = frame_size;
> +
> +	for (i = 0; i < dev->info->port_cnt; i++)
> +		max_frame = max(max_frame, dev->ports[i].max_frame);

You can do what other switches do, and instead of caching into an array,
"return 0" for everything except the CPU port. The CPU port will always
be programmed with the largest MTU.

> +
> +	switch (dev->chip_id) {
> +	case KSZ8795_CHIP_ID:
> +	case KSZ8794_CHIP_ID:
> +	case KSZ8765_CHIP_ID:
> +		return ksz8795_change_mtu(dev, port, max_frame);
> +	case KSZ8830_CHIP_ID:
> +		return ksz8863_change_mtu(dev, port, max_frame);
> +	}
> +
> +	return -EOPNOTSUPP;
> +}
> +