linux-kernel - Re: [PATCH net-next v2 1/2] net/ethernet/realtek: Add Realtek automotive PCIe driver code

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: <8746dad6-a6f1-4db0-958b-7b66d9dbd1f5@lunn.ch>
Date:   Thu, 10 Aug 2023 16:41:08 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Justin Lai <justinlai0215@...ltek.com>
Cc:     kuba@...nel.org, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com, linux-kernel@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 1/2] net/ethernet/realtek: Add Realtek
 automotive PCIe driver code

> +#include <linux/version.h>
> +#include <linux/ethtool.h>
> +
> +#define RTL_ALLOC_SKB_INTR(napi, length) napi_alloc_skb(&(napi), length)
> +
> +#define NETIF_F_ALL_CSUM NETIF_F_CSUM_MASK
> +
> +#define NETIF_F_HW_VLAN_RX NETIF_F_HW_VLAN_CTAG_RX
> +#define NETIF_F_HW_VLAN_TX NETIF_F_HW_VLAN_CTAG_TX
> +
> +#define CONFIG_SRIOV 1
> +
> +#ifndef NETIF_F_RXALL
> +#define NETIF_F_RXALL 0u
> +#endif
> +
> +#ifndef NETIF_F_RXFCS
> +#define NETIF_F_RXFCS 0u
> +#endif
> +
> +#ifndef SET_NETDEV_DEV
> +#define SET_NETDEV_DEV(net, pdev)
> +#endif
> +
> +#ifndef SET_MODULE_OWNER
> +#define SET_MODULE_OWNER(dev)
> +#endif
> +
> +#ifndef SA_SHIRQ
> +#define SA_SHIRQ IRQF_SHARED
> +#endif
> +
> +#ifndef NETIF_F_GSO
> +#define gso_size tso_size
> +#define gso_segs tso_segs
> +#endif
> +
> +#ifndef dma_mapping_error
> +#define dma_mapping_error(a, b) 0
> +#endif
> +
> +#ifndef netif_err
> +#define netif_err(a, b, c, d)
> +#endif
> +
> +#ifndef FALSE
> +#define FALSE 0
> +#endif
> +
> +#ifndef TRUE
> +#define TRUE 1
> +#endif
> +
> +#ifndef false
> +#define false 0
> +#endif
> +
> +#ifndef true
> +#define true 1
> +#endif

When i see code like this, it just shouts 'vendor crap, don't bother
reviewing'.

Really, truly, get help from an experienced mainline developer to
rewrite this code to mainline quality. Then post version 3.

Just as a hint, you are targeting net-next/main, and only
net-next/main. You can and should use everything which is in
net-next/main, and you should assume it exists. You are not targeting
older kernels, and you should not have 'vendor crap' like this so it
will compile with older kernels.

Spend some time looking at other drivers in mainline. If you are doing
something which other driver don't do, very likely you are doing
something wrong. Do you see other drivers looking to see if
NETIF_F_RXALL exists, and it not setting it to 0?

And please don't just fix this and repost. There is a lot more wrong.
Find a mentor to help you. The community would like to see this driver
in the kernel, but an entity the size of Realtek can easily contract
somebody to help get the code into shape.

    Andrew

---
pw-bot: cr