linux-kernel - RE: [PATCH net-next v1 1/6] lan743x: boost performance on cpu archs w/o dma cache snooping

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]

Message-ID: <MN2PR11MB3662C6C13B2D549E339D7DD1FAB89@MN2PR11MB3662.namprd11.prod.outlook.com>
Date:   Sat, 30 Jan 2021 22:10:52 +0000
From:   <Bryan.Whitehead@...rochip.com>
To:     <thesven73@...il.com>, <UNGLinuxDriver@...rochip.com>,
        <davem@...emloft.net>, <kuba@...nel.org>
CC:     <andrew@...n.ch>, <rtgbnm@...il.com>, <sbauer@...ckbox.su>,
        <tharvey@...eworks.com>, <anders@...ningen.priv.no>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net-next v1 1/6] lan743x: boost performance on cpu archs
 w/o dma cache snooping

Sven, see below comments

> @@ -2148,11 +2149,18 @@ static int lan743x_rx_process_packet(struct
> lan743x_rx *rx)
>                         descriptor = &rx->ring_cpu_ptr[first_index];
> 
>                         /* unmap from dma */
> +                       packet_length = RX_DESC_DATA0_FRAME_LENGTH_GET_
> +                                       (descriptor->data0);
It appears you moved this packet_length assignment from just below the following if block, however  you left out the le32_to_cpu.See next comment

>                         if (buffer_info->dma_ptr) {
> -                               dma_unmap_single(&rx->adapter->pdev->dev,
> -                                                buffer_info->dma_ptr,
> -                                                buffer_info->buffer_length,
> -                                                DMA_FROM_DEVICE);
> +                               dma_sync_single_for_cpu(&rx->adapter->pdev->dev,
> +                                                       buffer_info->dma_ptr,
> +                                                       packet_length,
> +                                                       DMA_FROM_DEVICE);
> +                               dma_unmap_single_attrs(&rx->adapter->pdev->dev,
> +                                                      buffer_info->dma_ptr,
> +                                                      buffer_info->buffer_length,
> +                                                      DMA_FROM_DEVICE,
> +
> + DMA_ATTR_SKIP_CPU_SYNC);
>                                 buffer_info->dma_ptr = 0;
>                                 buffer_info->buffer_length = 0;
>                         }
Just below here is the following line
		packet_length = RX_DESC_DATA0_FRAME_LENGTH_GET_
				(le32_to_cpu(descriptor->data0));
This line was moved above the previous if block, but the le32_to_cpu was removed. Is that intentional?
Also I don't see any mention of this packet_length assignment (after the if block) being removed.
Since packet_length already contains this value, it seems unnecessary to keep this assignment.

> @@ -2167,8 +2175,8 @@ static int lan743x_rx_process_packet(struct
> lan743x_rx *rx)
>                         int index = first_index;
> 
>                         /* multi buffer packet not supported */
> -                       /* this should not happen since
> -                        * buffers are allocated to be at least jumbo size
> +                       /* this should not happen since buffers are allocated
> +                        * to be at least the mtu size configured in the mac.
>                          */
> 
>                         /* clean up buffers */ @@ -2628,6 +2636,9 @@ static int
> lan743x_netdev_change_mtu(struct net_device *netdev, int new_mtu)
>         struct lan743x_adapter *adapter = netdev_priv(netdev);
>         int ret = 0;
> 
> +       if (netif_running(netdev))
> +               return -EBUSY;
> +
>         ret = lan743x_mac_set_mtu(adapter, new_mtu);
>         if (!ret)
>                 netdev->mtu = new_mtu;
> --
> 2.17.1