[v2,01/10] appletalk: make localtalk and ppp support conditional
Commit Message
From: Arnd Bergmann <arnd@arndb.de>
The last localtalk driver is gone now, and ppp support was never fully
merged, but the code to support them for phase1 networking still calls
the deprecated .ndo_do_ioctl() helper.
In order to better isolate the localtalk and ppp portions of appletalk,
guard all of the corresponding code with CONFIG_DEV_APPLETALK checks,
including a preprocessor conditional that guards the internal ioctl calls.
This is currently all dead code and will now be left out of the
module since this Kconfig symbol is always undefined, but there are
plans to add a new driver for localtalk again in the future. When
that happens, the logic can be cleaned up to work properly without
the need for the ioctl.
Link: https://lore.kernel.org/lkml/790BA488-B6F6-41ED-96EF-2089EF1C043B@xhero.org/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: only add compile-time consitionals rather than removing localtalk
support entirely
---
include/linux/atalk.h | 1 -
net/appletalk/Makefile | 3 ++-
net/appletalk/aarp.c | 24 +++++++++++++++---------
net/appletalk/ddp.c | 24 +++++++++++++-----------
4 files changed, 30 insertions(+), 22 deletions(-)
Comments
Could you provide a cover letter for the set please?
On Wed, Oct 11, 2023, at 17:04, Jiri Pirko wrote:
> Could you provide a cover letter for the set please?
Subject: [PATCH v2 00/10] remove final .ndo_do_ioctl references
The .ndo_do_ioctl() netdev operation used to be how one communicates
with a network driver from userspace, but since my previous cleanup [1],
it is purely internal to the kernel.
Removing the cops appletalk/localtalk driver made me revisit the
missing pieces from that older series, removing all the unused
implementations in wireless drivers as well as the two kernel-internal
callers in the ieee802154 and appletalk stacks.
One ethernet driver was already merged in the meantime that should
have used .ndo_eth_ioctl instead of .ndo_do_ioctl, so fix that as well.
With the complete removal, any future drivers making this mistake
cause build failures that are easier to spot.
[1] https://lore.kernel.org/netdev/20201106221743.3271965-1-arnd@kernel.org/
----
Hope that helps, I had commented on the cops removal about sending
this but of course not everyone here saw that. Let me know if I should
resend the patches together with the cover letter.
Arnd
Wed, Oct 11, 2023 at 05:57:38PM CEST, arnd@arndb.de wrote:
>On Wed, Oct 11, 2023, at 17:04, Jiri Pirko wrote:
>> Could you provide a cover letter for the set please?
>
>Subject: [PATCH v2 00/10] remove final .ndo_do_ioctl references
>
>The .ndo_do_ioctl() netdev operation used to be how one communicates
>with a network driver from userspace, but since my previous cleanup [1],
>it is purely internal to the kernel.
>
>Removing the cops appletalk/localtalk driver made me revisit the
>missing pieces from that older series, removing all the unused
>implementations in wireless drivers as well as the two kernel-internal
>callers in the ieee802154 and appletalk stacks.
>
>One ethernet driver was already merged in the meantime that should
>have used .ndo_eth_ioctl instead of .ndo_do_ioctl, so fix that as well.
>With the complete removal, any future drivers making this mistake
>cause build failures that are easier to spot.
Looks fine.
>
>[1] https://lore.kernel.org/netdev/20201106221743.3271965-1-arnd@kernel.org/
>
>----
>Hope that helps, I had commented on the cops removal about sending
>this but of course not everyone here saw that. Let me know if I should
>resend the patches together with the cover letter.
Yes please. Thanks!
>
> Arnd
On Wed, 11 Oct 2023 17:57:38 +0200 Arnd Bergmann wrote:
> The .ndo_do_ioctl() netdev operation used to be how one communicates
> with a network driver from userspace, but since my previous cleanup [1],
> it is purely internal to the kernel.
>
> Removing the cops appletalk/localtalk driver made me revisit the
> missing pieces from that older series, removing all the unused
> implementations in wireless drivers as well as the two kernel-internal
> callers in the ieee802154 and appletalk stacks.
>
> One ethernet driver was already merged in the meantime that should
> have used .ndo_eth_ioctl instead of .ndo_do_ioctl, so fix that as well.
> With the complete removal, any future drivers making this mistake
> cause build failures that are easier to spot.
>
> [1] https://lore.kernel.org/netdev/20201106221743.3271965-1-arnd@kernel.org/
Kalle, Johannes, do these apply for you?
I'm getting a small conflict on patch 8 and bigger one on patch 9.
If this applies for you maybe you can take it and flush out
wireless-next soon after?
Jakub Kicinski <kuba@kernel.org> writes:
> On Wed, 11 Oct 2023 17:57:38 +0200 Arnd Bergmann wrote:
>> The .ndo_do_ioctl() netdev operation used to be how one communicates
>> with a network driver from userspace, but since my previous cleanup [1],
>> it is purely internal to the kernel.
>>
>> Removing the cops appletalk/localtalk driver made me revisit the
>> missing pieces from that older series, removing all the unused
>> implementations in wireless drivers as well as the two kernel-internal
>> callers in the ieee802154 and appletalk stacks.
>>
>> One ethernet driver was already merged in the meantime that should
>> have used .ndo_eth_ioctl instead of .ndo_do_ioctl, so fix that as well.
>> With the complete removal, any future drivers making this mistake
>> cause build failures that are easier to spot.
>>
>> [1] https://lore.kernel.org/netdev/20201106221743.3271965-1-arnd@kernel.org/
>
> Kalle, Johannes, do these apply for you?
> I'm getting a small conflict on patch 8 and bigger one on patch 9.
Yes, 'git am -3' was able to solve the conflicts automatically and these
do apply to wireless-next.
> If this applies for you maybe you can take it and flush out
> wireless-next soon after?
Ok, we'll do that. Does next Wednesday sound soon enough? :)
On Wed, 11 Oct 2023 16:02:16 +0200 Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> The last localtalk driver is gone now, and ppp support was never fully
> merged, but the code to support them for phase1 networking still calls
> the deprecated .ndo_do_ioctl() helper.
>
> In order to better isolate the localtalk and ppp portions of appletalk,
> guard all of the corresponding code with CONFIG_DEV_APPLETALK checks,
> including a preprocessor conditional that guards the internal ioctl calls.
>
> This is currently all dead code and will now be left out of the
> module since this Kconfig symbol is always undefined, but there are
> plans to add a new driver for localtalk again in the future. When
> that happens, the logic can be cleaned up to work properly without
> the need for the ioctl.
Hi Arnd, the WiFi changes are now in net, could you rebase & repost?
On Tue, 17 Oct 2023 17:22:02 -0700 Jakub Kicinski wrote:
> Hi Arnd, the WiFi changes are now in net, could you rebase & repost?
s/net/net-next/ to be more clear this time..
@@ -121,7 +121,6 @@ static inline struct atalk_iface *atalk_find_dev(struct net_device *dev)
#endif
extern struct atalk_addr *atalk_find_dev_addr(struct net_device *dev);
-extern struct net_device *atrtr_get_dev(struct atalk_addr *sa);
extern int aarp_send_ddp(struct net_device *dev,
struct sk_buff *skb,
struct atalk_addr *sa, void *hwaddr);
@@ -5,6 +5,7 @@
obj-$(CONFIG_ATALK) += appletalk.o
-appletalk-y := aarp.o ddp.o dev.o
+appletalk-y := aarp.o ddp.o
appletalk-$(CONFIG_PROC_FS) += atalk_proc.o
appletalk-$(CONFIG_SYSCTL) += sysctl_net_atalk.o
+appletalk-$(CONFIG_DEV_APPLETALK) += dev.o
@@ -438,14 +438,17 @@ static struct atalk_addr *__aarp_proxy_find(struct net_device *dev,
*/
static void aarp_send_probe_phase1(struct atalk_iface *iface)
{
+#if IS_ENABLED(CONFIG_DEV_APPLETALK)
struct ifreq atreq;
struct sockaddr_at *sa = (struct sockaddr_at *)&atreq.ifr_addr;
const struct net_device_ops *ops = iface->dev->netdev_ops;
sa->sat_addr.s_node = iface->address.s_node;
sa->sat_addr.s_net = ntohs(iface->address.s_net);
-
- /* We pass the Net:Node to the drivers/cards by a Device ioctl. */
+ /*
+ * We used to pass the address via device ioctl, this has to
+ * be rewritten if we bring back localtalk.
+ */
if (!(ops->ndo_do_ioctl(iface->dev, &atreq, SIOCSIFADDR))) {
ops->ndo_do_ioctl(iface->dev, &atreq, SIOCGIFADDR);
if (iface->address.s_net != htons(sa->sat_addr.s_net) ||
@@ -455,13 +458,15 @@ static void aarp_send_probe_phase1(struct atalk_iface *iface)
iface->address.s_net = htons(sa->sat_addr.s_net);
iface->address.s_node = sa->sat_addr.s_node;
}
+#endif
}
void aarp_probe_network(struct atalk_iface *atif)
{
- if (atif->dev->type == ARPHRD_LOCALTLK ||
- atif->dev->type == ARPHRD_PPP)
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ (atif->dev->type == ARPHRD_LOCALTLK ||
+ atif->dev->type == ARPHRD_PPP))
aarp_send_probe_phase1(atif);
else {
unsigned int count;
@@ -488,8 +493,9 @@ int aarp_proxy_probe_network(struct atalk_iface *atif, struct atalk_addr *sa)
* we don't currently support LocalTalk or PPP for proxy AARP;
* if someone wants to try and add it, have fun
*/
- if (atif->dev->type == ARPHRD_LOCALTLK ||
- atif->dev->type == ARPHRD_PPP)
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ (atif->dev->type == ARPHRD_LOCALTLK ||
+ atif->dev->type == ARPHRD_PPP))
goto out;
/*
@@ -550,7 +556,8 @@ int aarp_send_ddp(struct net_device *dev, struct sk_buff *skb,
skb_reset_network_header(skb);
/* Check for LocalTalk first */
- if (dev->type == ARPHRD_LOCALTLK) {
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) &&
+ dev->type == ARPHRD_LOCALTLK) {
struct atalk_addr *at = atalk_find_dev_addr(dev);
struct ddpehdr *ddp = (struct ddpehdr *)skb->data;
int ft = 2;
@@ -588,7 +595,7 @@ int aarp_send_ddp(struct net_device *dev, struct sk_buff *skb,
}
/* On a PPP link we neither compress nor aarp. */
- if (dev->type == ARPHRD_PPP) {
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK) && dev->type == ARPHRD_PPP) {
skb->protocol = htons(ETH_P_PPPTALK);
skb->dev = dev;
goto sendit;
@@ -674,7 +681,6 @@ int aarp_send_ddp(struct net_device *dev, struct sk_buff *skb,
drop:
return NET_XMIT_DROP;
}
-EXPORT_SYMBOL(aarp_send_ddp);
/*
* An entry in the aarp unresolved queue has become resolved. Send
@@ -473,7 +473,7 @@ static struct atalk_route *atrtr_find(struct atalk_addr *target)
* Given an AppleTalk network, find the device to use. This can be
* a simple lookup.
*/
-struct net_device *atrtr_get_dev(struct atalk_addr *sa)
+static struct net_device *atrtr_get_dev(struct atalk_addr *sa)
{
struct atalk_route *atr = atrtr_find(sa);
return atr ? atr->dev : NULL;
@@ -1947,10 +1947,6 @@ static struct packet_type ppptalk_packet_type __read_mostly = {
static unsigned char ddp_snap_id[] = { 0x08, 0x00, 0x07, 0x80, 0x9B };
-/* Export symbols for use by drivers when AppleTalk is a module */
-EXPORT_SYMBOL(atrtr_get_dev);
-EXPORT_SYMBOL(atalk_find_dev_addr);
-
/* Called by proto.c on kernel start up */
static int __init atalk_init(void)
{
@@ -1971,8 +1967,10 @@ static int __init atalk_init(void)
goto out_sock;
}
- dev_add_pack(<alk_packet_type);
- dev_add_pack(&ppptalk_packet_type);
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK)) {
+ dev_add_pack(<alk_packet_type);
+ dev_add_pack(&ppptalk_packet_type);
+ }
rc = register_netdevice_notifier(&ddp_notifier);
if (rc)
@@ -1998,8 +1996,10 @@ static int __init atalk_init(void)
out_dev:
unregister_netdevice_notifier(&ddp_notifier);
out_snap:
- dev_remove_pack(&ppptalk_packet_type);
- dev_remove_pack(<alk_packet_type);
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK)) {
+ dev_remove_pack(&ppptalk_packet_type);
+ dev_remove_pack(<alk_packet_type);
+ }
unregister_snap_client(ddp_dl);
out_sock:
sock_unregister(PF_APPLETALK);
@@ -2026,8 +2026,10 @@ static void __exit atalk_exit(void)
atalk_proc_exit();
aarp_cleanup_module(); /* General aarp clean-up. */
unregister_netdevice_notifier(&ddp_notifier);
- dev_remove_pack(<alk_packet_type);
- dev_remove_pack(&ppptalk_packet_type);
+ if (IS_ENABLED(CONFIG_DEV_APPLETALK)) {
+ dev_remove_pack(<alk_packet_type);
+ dev_remove_pack(&ppptalk_packet_type);
+ }
unregister_snap_client(ddp_dl);
sock_unregister(PF_APPLETALK);
proto_unregister(&ddp_proto);