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: <3135e933-09cb-e397-972b-d66c48bbf772@infradead.org>
Date: Wed, 28 Jun 2023 22:12:32 -0700
From: Randy Dunlap <rdunlap@...radead.org>
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: Simon Horman <simon.horman@...igine.com>,
 Christian Borntraeger <borntraeger@...ux.ibm.com>, netdev@...r.kernel.org,
 linux-s390@...r.kernel.org, Heiko Carstens <hca@...ux.ibm.com>
Subject: Re: [PATCH] s390/lcs: Remove FDDI option



On 6/28/23 06:57, Alexandra Winter wrote:
> 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>


Reviewed-by: Randy Dunlap <rdunlap@...radead.org>

Thanks.

> ---
>  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");

-- 
~Randy