netdev - Re: [PATCH] s390/lcs: Remove FDDI option

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: <6282d9ad-59b0-5648-74da-7923b07843b4@linux.ibm.com>
Date:   Wed, 28 Jun 2023 16:01:09 +0200
From:   Christian Borntraeger <borntraeger@...ux.ibm.com>
To:     Alexandra Winter <wintera@...ux.ibm.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <edumazet@...gle.com>
Cc:     Randy Dunlap <rdunlap@...radead.org>,
        Simon Horman <simon.horman@...igine.com>,
        netdev@...r.kernel.org, linux-s390@...r.kernel.org,
        Heiko Carstens <hca@...ux.ibm.com>
Subject: Re: [PATCH] s390/lcs: Remove FDDI option

Am 28.06.23 um 15:57 schrieb Alexandra Winter:
> The last s390 machine that supported FDDI was z900 ('7th generation',
> released in 2000). The oldest machine generation currently supported by
> the Linux kernel is MARCH_Z10 (released 2008). If there is still a usecase
> for connecting a Linux on s390 instance to a LAN Channel Station (LCS), it
> can only do so via Ethernet.
> 
> Randy Dunlap[1] found that LCS over FDDI has never worked, when FDDI
> was compiled as module. Instead of fixing that, remove the FDDI option
> from the lcs driver.
> 
> While at it, make the CONFIG_LCS description a bit more helpful.
> 
> References:
> [1] https://lore.kernel.org/netdev/20230621213742.8245-1-rdunlap@infradead.org/
> 
> Signed-off-by: Alexandra Winter <wintera@...ux.ibm.com>

Makes perfect sense given the complexity.

Acked-by: Christian Borntraeger <borntraeger@...ux.ibm.com>


> ---
>   drivers/s390/net/Kconfig |  5 ++---
>   drivers/s390/net/lcs.c   | 39 ++++++---------------------------------
>   2 files changed, 8 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/s390/net/Kconfig b/drivers/s390/net/Kconfig
> index 9c67b97faba2..74760c1a163b 100644
> --- a/drivers/s390/net/Kconfig
> +++ b/drivers/s390/net/Kconfig
> @@ -5,12 +5,11 @@ menu "S/390 network device drivers"
>   config LCS
>   	def_tristate m
>   	prompt "Lan Channel Station Interface"
> -	depends on CCW && NETDEVICES && (ETHERNET || FDDI)
> +	depends on CCW && NETDEVICES && ETHERNET
>   	help
>   	  Select this option if you want to use LCS networking on IBM System z.
> -	  This device driver supports FDDI (IEEE 802.7) and Ethernet.
>   	  To compile as a module, choose M. The module name is lcs.
> -	  If you do not know what it is, it's safe to choose Y.
> +	  If you do not use LCS, choose N.
>   
>   config CTCM
>   	def_tristate m
> diff --git a/drivers/s390/net/lcs.c b/drivers/s390/net/lcs.c
> index 9fd8e6f07a03..a1f2acd6fb8f 100644
> --- a/drivers/s390/net/lcs.c
> +++ b/drivers/s390/net/lcs.c
> @@ -17,7 +17,6 @@
>   #include <linux/if.h>
>   #include <linux/netdevice.h>
>   #include <linux/etherdevice.h>
> -#include <linux/fddidevice.h>
>   #include <linux/inetdevice.h>
>   #include <linux/in.h>
>   #include <linux/igmp.h>
> @@ -36,10 +35,6 @@
>   #include "lcs.h"
>   
>   
> -#if !defined(CONFIG_ETHERNET) && !defined(CONFIG_FDDI)
> -#error Cannot compile lcs.c without some net devices switched on.
> -#endif
> -
>   /*
>    * initialization string for output
>    */
> @@ -1601,19 +1596,11 @@ lcs_startlan_auto(struct lcs_card *card)
>   	int rc;
>   
>   	LCS_DBF_TEXT(2, trace, "strtauto");
> -#ifdef CONFIG_ETHERNET
>   	card->lan_type = LCS_FRAME_TYPE_ENET;
>   	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
>   	if (rc == 0)
>   		return 0;
>   
> -#endif
> -#ifdef CONFIG_FDDI
> -	card->lan_type = LCS_FRAME_TYPE_FDDI;
> -	rc = lcs_send_startlan(card, LCS_INITIATOR_TCPIP);
> -	if (rc == 0)
> -		return 0;
> -#endif
>   	return -EIO;
>   }
>   
> @@ -1806,22 +1793,16 @@ lcs_get_frames_cb(struct lcs_channel *channel, struct lcs_buffer *buffer)
>   			card->stats.rx_errors++;
>   			return;
>   		}
> -		/* What kind of frame is it? */
> -		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL) {
> -			/* Control frame. */
> +		if (lcs_hdr->type == LCS_FRAME_TYPE_CONTROL)
>   			lcs_get_control(card, (struct lcs_cmd *) lcs_hdr);
> -		} else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_TR ||
> -			   lcs_hdr->type == LCS_FRAME_TYPE_FDDI) {
> -			/* Normal network packet. */
> +		else if (lcs_hdr->type == LCS_FRAME_TYPE_ENET)
>   			lcs_get_skb(card, (char *)(lcs_hdr + 1),
>   				    lcs_hdr->offset - offset -
>   				    sizeof(struct lcs_header));
> -		} else {
> -			/* Unknown frame type. */
> -			; // FIXME: error message ?
> -		}
> -		/* Proceed to next frame. */
> +		else
> +			dev_info_once(&card->dev->dev,
> +				      "Unknown frame type %d\n",
> +				      lcs_hdr->type);
>   		offset = lcs_hdr->offset;
>   		lcs_hdr->offset = LCS_ILLEGAL_OFFSET;
>   		lcs_hdr = (struct lcs_header *) (buffer->data + offset);
> @@ -2140,18 +2121,10 @@ lcs_new_device(struct ccwgroup_device *ccwgdev)
>   		goto netdev_out;
>   	}
>   	switch (card->lan_type) {
> -#ifdef CONFIG_ETHERNET
>   	case LCS_FRAME_TYPE_ENET:
>   		card->lan_type_trans = eth_type_trans;
>   		dev = alloc_etherdev(0);
>   		break;
> -#endif
> -#ifdef CONFIG_FDDI
> -	case LCS_FRAME_TYPE_FDDI:
> -		card->lan_type_trans = fddi_type_trans;
> -		dev = alloc_fddidev(0);
> -		break;
> -#endif
>   	default:
>   		LCS_DBF_TEXT(3, setup, "errinit");
>   		pr_err(" Initialization failed\n");