| 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: <e0337cfa-9744-7451-1fef-7f43f3b6cbe0@ti.com>
Date: Tue, 2 Jan 2024 15:40:55 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Sanjuán García, Jorge
<Jorge.SanjuanGarcia@...gon.com>
CC: "davem@...emloft.net" <davem@...emloft.net>,
"edumazet@...gle.com"
<edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"netdev@...r.kernel.org"
<netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
<s-vadapalli@...com>
Subject: Re: [PATCH 2/3] net: ethernet: ti: am65-cpsw: Introduce rx_packet_max
member
Hello,
On 02-01-2024 13:49, Sanjuán García, Jorge wrote:
> The value written to the register AM65_CPSW_PORT_REG_RX_MAXLEN
> is currently fixed to what AM65_CPSW_MAX_PACKET_SIZE defines. This
> patch prepares the driver so that the maximum received frame length
> can be configured in future updates.
>
> Signed-off-by: Jorge Sanjuan Garcia <jorge.sanjuangarcia@...gon.com>
> ---
For patches which add new features, please use the subject prefix
[PATCH net-next].
> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++-----
> drivers/net/ethernet/ti/am65-cpsw-nuss.h | 2 ++
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 378d69b8cb14..a920146d7a60 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -151,9 +151,11 @@ static void am65_cpsw_port_set_sl_mac(struct am65_cpsw_port
> *slave,
>
> static void am65_cpsw_sl_ctl_reset(struct am65_cpsw_port *port)
> {
> + struct am65_cpsw_common *common = port->common;
> +
> cpsw_sl_reset(port->slave.mac_sl, 100);
> /* Max length register has to be restored after MAC SL reset */
> - writel(AM65_CPSW_MAX_PACKET_SIZE,
> + writel(common->rx_packet_max,
> port->port_base + AM65_CPSW_PORT_REG_RX_MAXLEN);
Prior to this patch, since the RX Packet size was hard-coded, it was
being set to the same value across all ports. However, please note that
there are per-port registers:
port->port_base + AM65_CPSW_PORT_REG_RX_MAXLEN
So, wouldn't it be better to define the "rx_packet_max" member within
"struct am65_cpsw_port", rather than "struct am65_cpsw_common"?
The same question applies to the following sections as well.
I went through patch 3/3 of this series and the device-tree property:
max-frame-size
is being used to fetch the value for "rx_packet_max". But, isn't
max-frame-size a port specific parameter? Shouldn't it then be stored as
a member of "struct am65_cpsw_port".
> }
>
> @@ -455,7 +457,7 @@ static int am65_cpsw_nuss_common_open(struct
> am65_cpsw_common *common)
> AM65_CPSW_CTL_VLAN_AWARE | AM65_CPSW_CTL_P0_RX_PAD,
> common->cpsw_base + AM65_CPSW_REG_CTL);
> /* Max length register */
> - writel(AM65_CPSW_MAX_PACKET_SIZE,
> + writel(common->rx_packet_max,
> host_p->port_base + AM65_CPSW_PORT_REG_RX_MAXLEN);
> /* set base flow_id */
> writel(common->rx_flow_id_base,
> @@ -507,7 +509,7 @@ static int am65_cpsw_nuss_common_open(struct
> am65_cpsw_common *common)
>
> for (i = 0; i < common->rx_chns.descs_num; i++) {
> skb = __netdev_alloc_skb_ip_align(NULL,
> - AM65_CPSW_MAX_PACKET_SIZE,
> + common->rx_packet_max,
> GFP_KERNEL);
> if (!skb) {
> ret = -ENOMEM;
> @@ -848,7 +850,7 @@ static int am65_cpsw_nuss_rx_packets(struct am65_cpsw_common
> *common,
>
> k3_cppi_desc_pool_free(rx_chn->desc_pool, desc_rx);
>
> - new_skb = netdev_alloc_skb_ip_align(ndev, AM65_CPSW_MAX_PACKET_SIZE);
> + new_skb = netdev_alloc_skb_ip_align(ndev, common->rx_packet_max);
> if (new_skb) {
> ndev_priv = netdev_priv(ndev);
> am65_cpsw_nuss_set_offload_fwd_mark(skb,
> ndev_priv->offload_fwd_mark);
> @@ -2196,7 +2198,7 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common
> *common, u32 port_idx)
> eth_hw_addr_set(port->ndev, port->slave.mac_addr);
>
> port->ndev->min_mtu = AM65_CPSW_MIN_PACKET_SIZE;
> - port->ndev->max_mtu = AM65_CPSW_MAX_PACKET_SIZE -
> + port->ndev->max_mtu = common->rx_packet_max -
> (VLAN_ETH_HLEN + ETH_FCS_LEN);
> port->ndev->hw_features = NETIF_F_SG |
> NETIF_F_RXCSUM |
> @@ -2926,6 +2928,7 @@ static int am65_cpsw_nuss_probe(struct platform_device *pdev)
> return -ENOENT;
>
> common->rx_flow_id_base = -1;
> + common->rx_packet_max = AM65_CPSW_MAX_PACKET_SIZE;
> init_completion(&common->tdown_complete);
> common->tx_ch_num = AM65_CPSW_DEFAULT_TX_CHNS;
> common->pf_p0_rx_ptype_rrobin = false;
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> index f3dad2ab9828..141160223d73 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.h
> @@ -130,6 +130,8 @@ struct am65_cpsw_common {
> u32 tx_ch_rate_msk;
> u32 rx_flow_id_base;
>
> + int rx_packet_max;
> +
> struct am65_cpsw_tx_chn tx_chns[AM65_CPSW_MAX_TX_QUEUES];
> struct completion tdown_complete;
> atomic_t tdown_cnt;
...
--
Regards,
Siddharth.
Powered by blists - more mailing lists