diff mbox series

[01/10] appletalk: remove localtalk and ppp support

Message ID 20231009141908.1767241-1-arnd@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [01/10] appletalk: remove localtalk and ppp support | expand

Checks

Context Check Description
netdev/series_format warning Series does not have a cover letter
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 1475 this patch: 22
netdev/cc_maintainers warning 5 maintainers not CCed: willemdebruijn.kernel@gmail.com jasowang@redhat.com mkl@pengutronix.de dhowells@redhat.com wenjia@linux.ibm.com
netdev/build_clang fail Errors and warnings before: 1388 this patch: 19
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 1501 this patch: 22
netdev/checkpatch warning WARNING: Possible repeated word: 'it' WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 5 this patch: 5
netdev/source_inline success Was 0 now: 0

Commit Message

Arnd Bergmann Oct. 9, 2023, 2:18 p.m. UTC
From: Arnd Bergmann <arnd@arndb.de>

The last localtalk driver is gone now, and ppp support was never fully
merged, so clean up the appletalk code by removing the obvious dead
code paths.

Notably, this removes one of the two callers of the old .ndo_do_ioctl()
callback that was abused for getting device addresses and is now
only used in the ieee802154 subsystem, which still uses the same trick.

The include/uapi/linux/if_ltalk.h header might still be required
for building userspace programs, but I made sure that debian code
search and the netatalk upstream have no references it it, so it
should be fine to remove.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/tun.c             |   3 -
 include/linux/atalk.h         |   1 -
 include/linux/if_ltalk.h      |   8 ---
 include/uapi/linux/if_ltalk.h |  10 ----
 net/appletalk/Makefile        |   2 +-
 net/appletalk/aarp.c          | 108 +++-------------------------------
 net/appletalk/ddp.c           |  97 +-----------------------------
 net/appletalk/dev.c           |  46 ---------------
 8 files changed, 11 insertions(+), 264 deletions(-)
 delete mode 100644 include/linux/if_ltalk.h
 delete mode 100644 include/uapi/linux/if_ltalk.h
 delete mode 100644 net/appletalk/dev.c

Comments

Rodolfo Zitellini Oct. 9, 2023, 4:49 p.m. UTC | #1
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The last localtalk driver is gone now, and ppp support was never fully
> merged, so clean up the appletalk code by removing the obvious dead
> code paths.
> 
> Notably, this removes one of the two callers of the old .ndo_do_ioctl()
> callback that was abused for getting device addresses and is now
> only used in the ieee802154 subsystem, which still uses the same trick.
> 
> The include/uapi/linux/if_ltalk.h header might still be required
> for building userspace programs, but I made sure that debian code
> search and the netatalk upstream have no references it it, so it
> should be fine to remove.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Hi!
I’ve been working on a new LocalTalk interface driver for the last couple months, do you think it would be possible to at least postpone the removal of LT a bit?

It is a driver for an open source device called TashTalk (https://github.com/lampmerchant/tashtalk), which runs on a PIC micro that does all the LT interfacing, and communicates back via serial to the host system. My driver is relatively simple and works very well with netatalk 2.2 (which is still maintained and still has support for AppleTalk). The driver is basically complete and trsted and I was preparing to submit a patch.

Still having LocalTalk in my view has many advantages for us enthusiasts that still want to bridge old machines to the current world without modifications, for example for printing on modern printers, netbooting, sharing files and even tcp/ip. All this basically works out of the box via the driver, Linux and available userspace tools (netatalk, macipgw).

The old ISA cards supported by COPS were basically unobtanium even 20 years ago, but the solution of using a PIC and a serial port is very robust and much more furure-proof. We also already have a device that can interface a modern machine directly via USB to LocalTalk.

The development of the TashTalk has been also extensively discussed on thr 68KMLA forum (https://68kmla.org/bb/index.php?threads/modtashtalk-lt0-driver-for-linux.45031/)

I hope the decision to remove LocalTalk can be reconsidered at least for the time being so there is a chance to submit a new, modern device making use of this stack.

Many Thanks,
Rodolfo Zitellini
Arnd Bergmann Oct. 9, 2023, 5:29 p.m. UTC | #2
On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote:
>> From: Arnd Bergmann <arnd@arndb.de>
>> 
>> The last localtalk driver is gone now, and ppp support was never fully
>> merged, so clean up the appletalk code by removing the obvious dead
>> code paths.
>> 
>> Notably, this removes one of the two callers of the old .ndo_do_ioctl()
>> callback that was abused for getting device addresses and is now
>> only used in the ieee802154 subsystem, which still uses the same trick.
>> 
>> The include/uapi/linux/if_ltalk.h header might still be required
>> for building userspace programs, but I made sure that debian code
>> search and the netatalk upstream have no references it it, so it
>> should be fine to remove.
>> 
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Hi!
> I’ve been working on a new LocalTalk interface driver for the last 
> couple months, do you think it would be possible to at least postpone 
> the removal of LT a bit?
>
> It is a driver for an open source device called TashTalk 
> (https://github.com/lampmerchant/tashtalk), which runs on a PIC micro 
> that does all the LT interfacing, and communicates back via serial to 
> the host system. My driver is relatively simple and works very well 
> with netatalk 2.2 (which is still maintained and still has support for 
> AppleTalk). The driver is basically complete and trsted and I was 
> preparing to submit a patch.
>
> Still having LocalTalk in my view has many advantages for us 
> enthusiasts that still want to bridge old machines to the current world 
> without modifications, for example for printing on modern printers, 
> netbooting, sharing files and even tcp/ip. All this basically works out 
> of the box via the driver, Linux and available userspace tools 
> (netatalk, macipgw).
>
> The old ISA cards supported by COPS were basically unobtanium even 20 
> years ago, but the solution of using a PIC and a serial port is very 
> robust and much more furure-proof. We also already have a device that 
> can interface a modern machine directly via USB to LocalTalk.
>
> The development of the TashTalk has been also extensively discussed on 
> thr 68KMLA forum 
> (https://68kmla.org/bb/index.php?threads/modtashtalk-lt0-driver-for-linux.45031/)
>
> I hope the decision to remove LocalTalk can be reconsidered at least 
> for the time being so there is a chance to submit a new, modern device 
> making use of this stack.

Nothing is decided, I'm just proposing my patch as a cleanup
for now. It would be nice to still drop the ndo_do_ioctl function
though, at least in some form. When your driver actually makes
it into the kernel, you can find a different method of communicating
the address between the socket interface and the device driver.

I can see a few ways this could work out:

- add a custom callback pointer to struct atalk_iface to
  get and set the address for phase1 probing instead of going
  through the ioctl

- rewrite the probing logic in aarp.c more widely, and improve
  the userspace interface in the process by introducing a netlink
  interface

- Move your entire driver into userspace and go to the kernel
  using tun/tap. This has the added benefit of avoiding a lot
  of the complexity of the tty line discipline code you have.

      Arnd
Stephen Hemminger Oct. 9, 2023, 11:43 p.m. UTC | #3
On Mon, 9 Oct 2023 18:49:43 +0200
Rodolfo Zitellini <rwz@xhero.org> wrote:

> Hi!
> I’ve been working on a new LocalTalk interface driver for the last couple months, do you think it would be possible to at least postpone the removal of LT a bit?
> 
> It is a driver for an open source device called TashTalk (https://github.com/lampmerchant/tashtalk), which runs on a PIC micro that does all the LT interfacing, and communicates back via serial to the host system. My driver is relatively simple and works very well with netatalk 2.2 (which is still maintained and still has support for AppleTalk). The driver is basically complete and trsted and I was preparing to submit a patch.
> 
> Still having LocalTalk in my view has many advantages for us enthusiasts that still want to bridge old machines to the current world without modifications, for example for printing on modern printers, netbooting, sharing files and even tcp/ip. All this basically works out of the box via the driver, Linux and available userspace tools (netatalk, macipgw).
> 
> The old ISA cards supported by COPS were basically unobtanium even 20 years ago, but the solution of using a PIC and a serial port is very robust and much more furure-proof. We also already have a device that can interface a modern machine directly via USB to LocalTalk.
> 
> The development of the TashTalk has been also extensively discussed on thr 68KMLA forum (https://68kmla.org/bb/index.php?threads/modtashtalk-lt0-driver-for-linux.45031/)
> 
> I hope the decision to remove LocalTalk can be reconsidered at least for the time being so there is a chance to submit a new, modern device making use of this stack.
> 
> Many Thanks,
> Rodolfo Zitellini

Does it really need it to be a kernel protocol stack?
What about doing it in userspace or with BPF?
Jakub Kicinski Oct. 10, 2023, 12:47 a.m. UTC | #4
On Mon,  9 Oct 2023 16:18:59 +0200 Arnd Bergmann wrote:
> The last localtalk driver is gone now, and ppp support was never fully
> merged, so clean up the appletalk code by removing the obvious dead
> code paths.
> 
> Notably, this removes one of the two callers of the old .ndo_do_ioctl()
> callback that was abused for getting device addresses and is now
> only used in the ieee802154 subsystem, which still uses the same trick.
> 
> The include/uapi/linux/if_ltalk.h header might still be required
> for building userspace programs, but I made sure that debian code
> search and the netatalk upstream have no references it it, so it
> should be fine to remove.

Looks like it depends on the ipddp driver removal.
Could you repost once that one is merged (~tomorrow)?
Rodolfo Zitellini Oct. 10, 2023, 7:10 a.m. UTC | #5
> Il giorno 9 ott 2023, alle ore 19:29, Arnd Bergmann <arnd@arndb.de> ha scritto:
> 
> On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote:
>>> From: Arnd Bergmann <arnd@arndb.de>
>>> 
>>> The last localtalk driver is gone now, and ppp support was never fully
>>> merged, so clean up the appletalk code by removing the obvious dead
>>> code paths.
>>> 
>>> Notably, this removes one of the two callers of the old .ndo_do_ioctl()
>>> callback that was abused for getting device addresses and is now
>>> only used in the ieee802154 subsystem, which still uses the same trick.
>>> 
>>> The include/uapi/linux/if_ltalk.h header might still be required
>>> for building userspace programs, but I made sure that debian code
>>> search and the netatalk upstream have no references it it, so it
>>> should be fine to remove.
>>> 
>>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> 
>> Hi!
>> I’ve been working on a new LocalTalk interface driver for the last 
>> couple months, do you think it would be possible to at least postpone 
>> the removal of LT a bit?
>> 
>> It is a driver for an open source device called TashTalk 
>> (https://github.com/lampmerchant/tashtalk), which runs on a PIC micro 
>> that does all the LT interfacing, and communicates back via serial to 
>> the host system. My driver is relatively simple and works very well 
>> with netatalk 2.2 (which is still maintained and still has support for 
>> AppleTalk). The driver is basically complete and trsted and I was 
>> preparing to submit a patch.
>> 
>> Still having LocalTalk in my view has many advantages for us 
>> enthusiasts that still want to bridge old machines to the current world 
>> without modifications, for example for printing on modern printers, 
>> netbooting, sharing files and even tcp/ip. All this basically works out 
>> of the box via the driver, Linux and available userspace tools 
>> (netatalk, macipgw).
>> 
>> The old ISA cards supported by COPS were basically unobtanium even 20 
>> years ago, but the solution of using a PIC and a serial port is very 
>> robust and much more furure-proof. We also already have a device that 
>> can interface a modern machine directly via USB to LocalTalk.
>> 
>> The development of the TashTalk has been also extensively discussed on 
>> thr 68KMLA forum 
>> (https://68kmla.org/bb/index.php?threads/modtashtalk-lt0-driver-for-linux.45031/)
>> 
>> I hope the decision to remove LocalTalk can be reconsidered at least 
>> for the time being so there is a chance to submit a new, modern device 
>> making use of this stack.
> 
> Nothing is decided, I'm just proposing my patch as a cleanup
> for now. It would be nice to still drop the ndo_do_ioctl function
> though, at least in some form. When your driver actually makes
> it into the kernel, you can find a different method of communicating
> the address between the socket interface and the device driver.

Yes I too think it is good to remove ndo_do_ioctl, I designed the TashTalk driver to be a drop-in replacement for COPS mostly for compatibility with netatalk 2.2. My plan was to propose it like this (so nothing else needed to be changed) and the propose some patches in the kernel part and userspace part (netatalk).

> I can see a few ways this could work out:
> 
> - add a custom callback pointer to struct atalk_iface to
>  get and set the address for phase1 probing instead of going
>  through the ioctl

This was my initial thought, at least for the moment, mostly to keep netatalk happy and make sure I don’t break other stuff that makes assumptions on how the address probing worked. There are other bits I would like to improve, for example tcpdump (which parses correctly appetalk packets!) is broken in the current implementation.

> - rewrite the probing logic in aarp.c more widely, and improve
>  the userspace interface in the process by introducing a netlink
>  interface

This is sorta the “second step” I was planning, I think the logic for probing could be redesigned and simplified (it also does not work 100% correctly), and it could be a good chance to improve the interface with netatalk too.

> - Move your entire driver into userspace and go to the kernel
>  using tun/tap. This has the added benefit of avoiding a lot
>  of the complexity of the tty line discipline code you have.

We had some discussion too if to just make the lt an userspace stack, I personally like how it is currently implemented because existing code can run basically without modification.

I would propose at this stage to change the TashTalk driver to remove ndo_do_ioctl and to use a custom callback, if this ok.

Many thanks,
Rodolfo
Arnd Bergmann Oct. 10, 2023, 8:15 a.m. UTC | #6
On Tue, Oct 10, 2023, at 09:10, Rodolfo Zitellini wrote:
>> Il giorno 9 ott 2023, alle ore 19:29, Arnd Bergmann <arnd@arndb.de> ha scritto:
>> On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote:
>> I can see a few ways this could work out:
>> 
>> - add a custom callback pointer to struct atalk_iface to
>>  get and set the address for phase1 probing instead of going
>>  through the ioctl
>
> This was my initial thought, at least for the moment, mostly to keep 
> netatalk happy and make sure I don’t break other stuff that makes 
> assumptions on how the address probing worked. There are other bits I 
> would like to improve, for example tcpdump (which parses correctly 
> appetalk packets!) is broken in the current implementation.
>
>> - rewrite the probing logic in aarp.c more widely, and improve
>>  the userspace interface in the process by introducing a netlink
>>  interface
>
> This is sorta the “second step” I was planning, I think the logic for 
> probing could be redesigned and simplified (it also does not work 100% 
> correctly), and it could be a good chance to improve the interface with 
> netatalk too.

Ok, I've adapted my patch now to not actually drop the
localtalk code for now, and sent that out, I hope that works
for you. Even if we go with the v1 patch that removes it all,
you could just as well start with a revert of my patch when
you add your driver, so in the end it shouldn't make much
of a difference.

>> - Move your entire driver into userspace and go to the kernel
>>  using tun/tap. This has the added benefit of avoiding a lot
>>  of the complexity of the tty line discipline code you have.
>
> We had some discussion too if to just make the lt an userspace stack, I 
> personally like how it is currently implemented because existing code 
> can run basically without modification.
>
> I would propose at this stage to change the TashTalk driver to remove 
> ndo_do_ioctl and to use a custom callback, if this ok.

It looks like you still need a custom userspace tool to set up
the uart for your new driver, so my feeling would be that having a
userspace bridge to implement the localtalk/uart to ethertalk/tap
driver would actually be nicer for both usability and maintenance.

It's not something we need to decide now though, and is up to
you in the end.

      Arnd
kernel test robot Oct. 10, 2023, 9:21 a.m. UTC | #7
Hi Arnd,

kernel test robot noticed the following build errors:

[auto build test ERROR on next-20231009]
[cannot apply to linus/master v6.6-rc5 v6.6-rc4 v6.6-rc3 v6.6-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/ieee802154-avoid-deprecated-ndo_do_ioctl-callback/20231009-222305
base:   next-20231009
patch link:    https://lore.kernel.org/r/20231009141908.1767241-1-arnd%40kernel.org
patch subject: [PATCH 01/10] appletalk: remove localtalk and ppp support
config: nios2-randconfig-001-20231010 (https://download.01.org/0day-ci/archive/20231010/202310101724.iRnAoP3r-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231010/202310101724.iRnAoP3r-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310101724.iRnAoP3r-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   drivers/net/appletalk/ipddp.c: In function 'ipddp_create':
>> drivers/net/appletalk/ipddp.c:207:24: error: implicit declaration of function 'atrtr_get_dev'; did you mean 'to_net_dev'? [-Werror=implicit-function-declaration]
     207 |         if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
         |                        ^~~~~~~~~~~~~
         |                        to_net_dev
>> drivers/net/appletalk/ipddp.c:207:22: warning: assignment to 'struct net_device *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
     207 |         if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
         |                      ^
   cc1: some warnings being treated as errors


vim +207 drivers/net/appletalk/ipddp.c

^1da177e4c3f41 Linus Torvalds   2005-04-16  192  
^1da177e4c3f41 Linus Torvalds   2005-04-16  193  /*
^1da177e4c3f41 Linus Torvalds   2005-04-16  194   * Create a routing entry. We first verify that the
^1da177e4c3f41 Linus Torvalds   2005-04-16  195   * record does not already exist. If it does we return -EEXIST
^1da177e4c3f41 Linus Torvalds   2005-04-16  196   */
^1da177e4c3f41 Linus Torvalds   2005-04-16  197  static int ipddp_create(struct ipddp_route *new_rt)
^1da177e4c3f41 Linus Torvalds   2005-04-16  198  {
ce7e40c432ba84 Vlad Tsyrklevich 2017-01-09  199          struct ipddp_route *rt = kzalloc(sizeof(*rt), GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds   2005-04-16  200  
^1da177e4c3f41 Linus Torvalds   2005-04-16  201          if (rt == NULL)
^1da177e4c3f41 Linus Torvalds   2005-04-16  202                  return -ENOMEM;
^1da177e4c3f41 Linus Torvalds   2005-04-16  203  
^1da177e4c3f41 Linus Torvalds   2005-04-16  204          rt->ip = new_rt->ip;
^1da177e4c3f41 Linus Torvalds   2005-04-16  205          rt->at = new_rt->at;
^1da177e4c3f41 Linus Torvalds   2005-04-16  206          rt->next = NULL;
^1da177e4c3f41 Linus Torvalds   2005-04-16 @207          if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
^1da177e4c3f41 Linus Torvalds   2005-04-16  208  		kfree(rt);
^1da177e4c3f41 Linus Torvalds   2005-04-16  209                  return -ENETUNREACH;
^1da177e4c3f41 Linus Torvalds   2005-04-16  210          }
^1da177e4c3f41 Linus Torvalds   2005-04-16  211  
5615968a708451 David S. Miller  2009-05-27  212  	spin_lock_bh(&ipddp_route_lock);
5615968a708451 David S. Miller  2009-05-27  213  	if (__ipddp_find_route(rt)) {
5615968a708451 David S. Miller  2009-05-27  214  		spin_unlock_bh(&ipddp_route_lock);
^1da177e4c3f41 Linus Torvalds   2005-04-16  215  		kfree(rt);
^1da177e4c3f41 Linus Torvalds   2005-04-16  216  		return -EEXIST;
^1da177e4c3f41 Linus Torvalds   2005-04-16  217  	}
^1da177e4c3f41 Linus Torvalds   2005-04-16  218  
^1da177e4c3f41 Linus Torvalds   2005-04-16  219          rt->next = ipddp_route_list;
^1da177e4c3f41 Linus Torvalds   2005-04-16  220          ipddp_route_list = rt;
^1da177e4c3f41 Linus Torvalds   2005-04-16  221  
5615968a708451 David S. Miller  2009-05-27  222  	spin_unlock_bh(&ipddp_route_lock);
5615968a708451 David S. Miller  2009-05-27  223  
^1da177e4c3f41 Linus Torvalds   2005-04-16  224          return 0;
^1da177e4c3f41 Linus Torvalds   2005-04-16  225  }
^1da177e4c3f41 Linus Torvalds   2005-04-16  226
Rodolfo Zitellini Oct. 11, 2023, 7:09 a.m. UTC | #8
> Il giorno 10 ott 2023, alle ore 10:15, Arnd Bergmann <arnd@arndb.de> ha scritto:
> 
> On Tue, Oct 10, 2023, at 09:10, Rodolfo Zitellini wrote:
>>> Il giorno 9 ott 2023, alle ore 19:29, Arnd Bergmann <arnd@arndb.de> ha scritto:
>>> On Mon, Oct 9, 2023, at 18:49, Rodolfo Zitellini wrote:
>>> I can see a few ways this could work out:
>>> 
>>> - add a custom callback pointer to struct atalk_iface to
>>> get and set the address for phase1 probing instead of going
>>> through the ioctl
>> 
>> This was my initial thought, at least for the moment, mostly to keep 
>> netatalk happy and make sure I don’t break other stuff that makes 
>> assumptions on how the address probing worked. There are other bits I 
>> would like to improve, for example tcpdump (which parses correctly 
>> appetalk packets!) is broken in the current implementation.
>> 
>>> - rewrite the probing logic in aarp.c more widely, and improve
>>> the userspace interface in the process by introducing a netlink
>>> interface
>> 
>> This is sorta the “second step” I was planning, I think the logic for 
>> probing could be redesigned and simplified (it also does not work 100% 
>> correctly), and it could be a good chance to improve the interface with 
>> netatalk too.
> 
> Ok, I've adapted my patch now to not actually drop the
> localtalk code for now, and sent that out, I hope that works
> for you. Even if we go with the v1 patch that removes it all,
> you could just as well start with a revert of my patch when
> you add your driver, so in the end it shouldn't make much
> of a difference.

Thank you very much! I will try to make my patch ready to be submitted soon, and I will add the proper reverts if needed.

>>> - Move your entire driver into userspace and go to the kernel
>>> using tun/tap. This has the added benefit of avoiding a lot
>>> of the complexity of the tty line discipline code you have.
>> 
>> We had some discussion too if to just make the lt an userspace stack, I 
>> personally like how it is currently implemented because existing code 
>> can run basically without modification.
>> 
>> I would propose at this stage to change the TashTalk driver to remove 
>> ndo_do_ioctl and to use a custom callback, if this ok.
> 
> It looks like you still need a custom userspace tool to set up
> the uart for your new driver, so my feeling would be that having a
> userspace bridge to implement the localtalk/uart to ethertalk/tap
> driver would actually be nicer for both usability and maintenance.
> 
> It's not something we need to decide now though, and is up to
> you in the end.

I will experiment with this too, as it will require a bit of work to morph localtalk packets to ethertalk/aarp ones, and the code in the kernel has some specialized bits for localtalk here and there.

In any case, many thanks!
Rodolfo
kernel test robot Oct. 11, 2023, 9:27 a.m. UTC | #9
Hi Arnd,

kernel test robot noticed the following build warnings:

[auto build test WARNING on next-20231009]
[cannot apply to linus/master v6.6-rc5 v6.6-rc4 v6.6-rc3 v6.6-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/ieee802154-avoid-deprecated-ndo_do_ioctl-callback/20231009-222305
base:   next-20231009
patch link:    https://lore.kernel.org/r/20231009141908.1767241-1-arnd%40kernel.org
patch subject: [PATCH 01/10] appletalk: remove localtalk and ppp support
config: x86_64-randconfig-002-20231011 (https://download.01.org/0day-ci/archive/20231011/202310111736.4mh6Cf5C-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231011/202310111736.4mh6Cf5C-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310111736.4mh6Cf5C-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/appletalk/ipddp.c: In function 'ipddp_create':
   drivers/net/appletalk/ipddp.c:207:24: error: implicit declaration of function 'atrtr_get_dev'; did you mean 'to_net_dev'? [-Werror=implicit-function-declaration]
            if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
                           ^~~~~~~~~~~~~
                           to_net_dev
>> drivers/net/appletalk/ipddp.c:207:22: warning: assignment makes pointer from integer without a cast [-Wint-conversion]
            if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
                         ^
   cc1: some warnings being treated as errors


vim +207 drivers/net/appletalk/ipddp.c

^1da177e4c3f41 Linus Torvalds   2005-04-16  192  
^1da177e4c3f41 Linus Torvalds   2005-04-16  193  /*
^1da177e4c3f41 Linus Torvalds   2005-04-16  194   * Create a routing entry. We first verify that the
^1da177e4c3f41 Linus Torvalds   2005-04-16  195   * record does not already exist. If it does we return -EEXIST
^1da177e4c3f41 Linus Torvalds   2005-04-16  196   */
^1da177e4c3f41 Linus Torvalds   2005-04-16  197  static int ipddp_create(struct ipddp_route *new_rt)
^1da177e4c3f41 Linus Torvalds   2005-04-16  198  {
ce7e40c432ba84 Vlad Tsyrklevich 2017-01-09  199          struct ipddp_route *rt = kzalloc(sizeof(*rt), GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds   2005-04-16  200  
^1da177e4c3f41 Linus Torvalds   2005-04-16  201          if (rt == NULL)
^1da177e4c3f41 Linus Torvalds   2005-04-16  202                  return -ENOMEM;
^1da177e4c3f41 Linus Torvalds   2005-04-16  203  
^1da177e4c3f41 Linus Torvalds   2005-04-16  204          rt->ip = new_rt->ip;
^1da177e4c3f41 Linus Torvalds   2005-04-16  205          rt->at = new_rt->at;
^1da177e4c3f41 Linus Torvalds   2005-04-16  206          rt->next = NULL;
^1da177e4c3f41 Linus Torvalds   2005-04-16 @207          if ((rt->dev = atrtr_get_dev(&rt->at)) == NULL) {
^1da177e4c3f41 Linus Torvalds   2005-04-16  208  		kfree(rt);
^1da177e4c3f41 Linus Torvalds   2005-04-16  209                  return -ENETUNREACH;
^1da177e4c3f41 Linus Torvalds   2005-04-16  210          }
^1da177e4c3f41 Linus Torvalds   2005-04-16  211  
5615968a708451 David S. Miller  2009-05-27  212  	spin_lock_bh(&ipddp_route_lock);
5615968a708451 David S. Miller  2009-05-27  213  	if (__ipddp_find_route(rt)) {
5615968a708451 David S. Miller  2009-05-27  214  		spin_unlock_bh(&ipddp_route_lock);
^1da177e4c3f41 Linus Torvalds   2005-04-16  215  		kfree(rt);
^1da177e4c3f41 Linus Torvalds   2005-04-16  216  		return -EEXIST;
^1da177e4c3f41 Linus Torvalds   2005-04-16  217  	}
^1da177e4c3f41 Linus Torvalds   2005-04-16  218  
^1da177e4c3f41 Linus Torvalds   2005-04-16  219          rt->next = ipddp_route_list;
^1da177e4c3f41 Linus Torvalds   2005-04-16  220          ipddp_route_list = rt;
^1da177e4c3f41 Linus Torvalds   2005-04-16  221  
5615968a708451 David S. Miller  2009-05-27  222  	spin_unlock_bh(&ipddp_route_lock);
5615968a708451 David S. Miller  2009-05-27  223  
^1da177e4c3f41 Linus Torvalds   2005-04-16  224          return 0;
^1da177e4c3f41 Linus Torvalds   2005-04-16  225  }
^1da177e4c3f41 Linus Torvalds   2005-04-16  226
diff mbox series

Patch

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 89ab9efe522c3..e11476296e253 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -70,7 +70,6 @@ 
 #include <linux/bpf_trace.h>
 #include <linux/mutex.h>
 #include <linux/ieee802154.h>
-#include <linux/if_ltalk.h>
 #include <uapi/linux/if_fddi.h>
 #include <uapi/linux/if_hippi.h>
 #include <uapi/linux/if_fc.h>
@@ -3059,8 +3058,6 @@  static unsigned char tun_get_addr_len(unsigned short type)
 		return ROSE_ADDR_LEN;
 	case ARPHRD_NETROM:
 		return AX25_ADDR_LEN;
-	case ARPHRD_LOCALTLK:
-		return LTALK_ALEN;
 	default:
 		return 0;
 	}
diff --git a/include/linux/atalk.h b/include/linux/atalk.h
index a55bfc6567d01..2896f2ac9568e 100644
--- a/include/linux/atalk.h
+++ b/include/linux/atalk.h
@@ -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);
diff --git a/include/linux/if_ltalk.h b/include/linux/if_ltalk.h
deleted file mode 100644
index 4cc1c0b778700..0000000000000
--- a/include/linux/if_ltalk.h
+++ /dev/null
@@ -1,8 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __LINUX_LTALK_H
-#define __LINUX_LTALK_H
-
-#include <uapi/linux/if_ltalk.h>
-
-extern struct net_device *alloc_ltalkdev(int sizeof_priv);
-#endif
diff --git a/include/uapi/linux/if_ltalk.h b/include/uapi/linux/if_ltalk.h
deleted file mode 100644
index fa61e776f598d..0000000000000
--- a/include/uapi/linux/if_ltalk.h
+++ /dev/null
@@ -1,10 +0,0 @@ 
-/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
-#ifndef _UAPI__LINUX_LTALK_H
-#define _UAPI__LINUX_LTALK_H
-
-#define LTALK_HLEN		1
-#define LTALK_MTU		600
-#define LTALK_ALEN		1
-
-
-#endif /* _UAPI__LINUX_LTALK_H */
diff --git a/net/appletalk/Makefile b/net/appletalk/Makefile
index 33164d972d379..152312a151800 100644
--- a/net/appletalk/Makefile
+++ b/net/appletalk/Makefile
@@ -5,6 +5,6 @@ 
 
 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
diff --git a/net/appletalk/aarp.c b/net/appletalk/aarp.c
index 9fa0b246902be..dfcd9f46cb3a6 100644
--- a/net/appletalk/aarp.c
+++ b/net/appletalk/aarp.c
@@ -432,49 +432,18 @@  static struct atalk_addr *__aarp_proxy_find(struct net_device *dev,
 	return a ? sa : NULL;
 }
 
-/*
- * Probe a Phase 1 device or a device that requires its Net:Node to
- * be set via an ioctl.
- */
-static void aarp_send_probe_phase1(struct atalk_iface *iface)
-{
-	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. */
-	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) ||
-		    iface->address.s_node != sa->sat_addr.s_node)
-			iface->status |= ATIF_PROBE_FAIL;
-
-		iface->address.s_net  = htons(sa->sat_addr.s_net);
-		iface->address.s_node = sa->sat_addr.s_node;
-	}
-}
-
-
 void aarp_probe_network(struct atalk_iface *atif)
 {
-	if (atif->dev->type == ARPHRD_LOCALTLK ||
-	    atif->dev->type == ARPHRD_PPP)
-		aarp_send_probe_phase1(atif);
-	else {
-		unsigned int count;
+	unsigned int count;
 
-		for (count = 0; count < AARP_RETRANSMIT_LIMIT; count++) {
-			aarp_send_probe(atif->dev, &atif->address);
+	for (count = 0; count < AARP_RETRANSMIT_LIMIT; count++) {
+		aarp_send_probe(atif->dev, &atif->address);
 
-			/* Defer 1/10th */
-			msleep(100);
+		/* Defer 1/10th */
+		msleep(100);
 
-			if (atif->status & ATIF_PROBE_FAIL)
-				break;
-		}
+		if (atif->status & ATIF_PROBE_FAIL)
+			break;
 	}
 }
 
@@ -484,14 +453,6 @@  int aarp_proxy_probe_network(struct atalk_iface *atif, struct atalk_addr *sa)
 	struct aarp_entry *entry;
 	unsigned int count;
 
-	/*
-	 * 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)
-		goto out;
-
 	/*
 	 * create a new AARP entry with the flags set to be published --
 	 * we need this one to hang around even if it's in use
@@ -549,51 +510,6 @@  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) {
-		struct atalk_addr *at = atalk_find_dev_addr(dev);
-		struct ddpehdr *ddp = (struct ddpehdr *)skb->data;
-		int ft = 2;
-
-		/*
-		 * Compressible ?
-		 *
-		 * IFF: src_net == dest_net == device_net
-		 * (zero matches anything)
-		 */
-
-		if ((!ddp->deh_snet || at->s_net == ddp->deh_snet) &&
-		    (!ddp->deh_dnet || at->s_net == ddp->deh_dnet)) {
-			skb_pull(skb, sizeof(*ddp) - 4);
-
-			/*
-			 *	The upper two remaining bytes are the port
-			 *	numbers	we just happen to need. Now put the
-			 *	length in the lower two.
-			 */
-			*((__be16 *)skb->data) = htons(skb->len);
-			ft = 1;
-		}
-		/*
-		 * Nice and easy. No AARP type protocols occur here so we can
-		 * just shovel it out with a 3 byte LLAP header
-		 */
-
-		skb_push(skb, 3);
-		skb->data[0] = sa->s_node;
-		skb->data[1] = at->s_node;
-		skb->data[2] = ft;
-		skb->dev     = dev;
-		goto sendit;
-	}
-
-	/* On a PPP link we neither compress nor aarp.  */
-	if (dev->type == ARPHRD_PPP) {
-		skb->protocol = htons(ETH_P_PPPTALK);
-		skb->dev = dev;
-		goto sendit;
-	}
-
 	/* Non ELAP we cannot do. */
 	if (dev->type != ARPHRD_ETHER)
 		goto free_it;
@@ -659,22 +575,12 @@  int aarp_send_ddp(struct net_device *dev, struct sk_buff *skb,
 out_unlock:
 	write_unlock_bh(&aarp_lock);
 
-	/* Tell the ddp layer we have taken over for this frame. */
-	goto sent;
-
-sendit:
-	if (skb->sk)
-		skb->priority = READ_ONCE(skb->sk->sk_priority);
-	if (dev_queue_xmit(skb))
-		goto drop;
 sent:
 	return NET_XMIT_SUCCESS;
 free_it:
 	kfree_skb(skb);
-drop:
 	return NET_XMIT_DROP;
 }
-EXPORT_SYMBOL(aarp_send_ddp);
 
 /*
  *	An entry in the aarp unresolved queue has become resolved. Send
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 8978fb6212ffb..9fe344b08dc8b 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -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;
@@ -683,9 +683,7 @@  static int atif_ioctl(int cmd, void __user *arg)
 		if (sa->sat_family != AF_APPLETALK)
 			return -EINVAL;
 		if (dev->type != ARPHRD_ETHER &&
-		    dev->type != ARPHRD_LOOPBACK &&
-		    dev->type != ARPHRD_LOCALTLK &&
-		    dev->type != ARPHRD_PPP)
+		    dev->type != ARPHRD_LOOPBACK)
 			return -EPROTONOSUPPORT;
 
 		nr = (struct atalk_netrange *)&sa->sat_zero[0];
@@ -1327,18 +1325,8 @@  static int atalk_route_packet(struct sk_buff *skb, struct net_device *dev,
 	 * Don't route multicast, etc., packets, or packets sent to "this
 	 * network"
 	 */
-	if (skb->pkt_type != PACKET_HOST || !ddp->deh_dnet) {
-		/*
-		 * FIXME:
-		 *
-		 * Can it ever happen that a packet is from a PPP iface and
-		 * needs to be broadcast onto the default network?
-		 */
-		if (dev->type == ARPHRD_PPP)
-			printk(KERN_DEBUG "AppleTalk: didn't forward broadcast "
-					  "packet received from PPP iface\n");
+	if (skb->pkt_type != PACKET_HOST || !ddp->deh_dnet)
 		goto free_it;
-	}
 
 	ta.s_net  = ddp->deh_dnet;
 	ta.s_node = ddp->deh_dnode;
@@ -1508,64 +1496,6 @@  static int atalk_rcv(struct sk_buff *skb, struct net_device *dev,
 
 }
 
-/*
- * Receive a LocalTalk frame. We make some demands on the caller here.
- * Caller must provide enough headroom on the packet to pull the short
- * header and append a long one.
- */
-static int ltalk_rcv(struct sk_buff *skb, struct net_device *dev,
-		     struct packet_type *pt, struct net_device *orig_dev)
-{
-	if (!net_eq(dev_net(dev), &init_net))
-		goto freeit;
-
-	/* Expand any short form frames */
-	if (skb_mac_header(skb)[2] == 1) {
-		struct ddpehdr *ddp;
-		/* Find our address */
-		struct atalk_addr *ap = atalk_find_dev_addr(dev);
-
-		if (!ap || skb->len < sizeof(__be16) || skb->len > 1023)
-			goto freeit;
-
-		/* Don't mangle buffer if shared */
-		if (!(skb = skb_share_check(skb, GFP_ATOMIC)))
-			return 0;
-
-		/*
-		 * The push leaves us with a ddephdr not an shdr, and
-		 * handily the port bytes in the right place preset.
-		 */
-		ddp = skb_push(skb, sizeof(*ddp) - 4);
-
-		/* Now fill in the long header */
-
-		/*
-		 * These two first. The mac overlays the new source/dest
-		 * network information so we MUST copy these before
-		 * we write the network numbers !
-		 */
-
-		ddp->deh_dnode = skb_mac_header(skb)[0];     /* From physical header */
-		ddp->deh_snode = skb_mac_header(skb)[1];     /* From physical header */
-
-		ddp->deh_dnet  = ap->s_net;	/* Network number */
-		ddp->deh_snet  = ap->s_net;
-		ddp->deh_sum   = 0;		/* No checksum */
-		/*
-		 * Not sure about this bit...
-		 */
-		/* Non routable, so force a drop if we slip up later */
-		ddp->deh_len_hops = htons(skb->len + (DDP_MAXHOPS << 10));
-	}
-	skb_reset_transport_header(skb);
-
-	return atalk_rcv(skb, dev, pt, orig_dev);
-freeit:
-	kfree_skb(skb);
-	return 0;
-}
-
 static int atalk_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 {
 	struct sock *sk = sock->sk;
@@ -1935,22 +1865,8 @@  static struct notifier_block ddp_notifier = {
 	.notifier_call	= ddp_device_event,
 };
 
-static struct packet_type ltalk_packet_type __read_mostly = {
-	.type		= cpu_to_be16(ETH_P_LOCALTALK),
-	.func		= ltalk_rcv,
-};
-
-static struct packet_type ppptalk_packet_type __read_mostly = {
-	.type		= cpu_to_be16(ETH_P_PPPTALK),
-	.func		= atalk_rcv,
-};
-
 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,9 +1887,6 @@  static int __init atalk_init(void)
 		goto out_sock;
 	}
 
-	dev_add_pack(&ltalk_packet_type);
-	dev_add_pack(&ppptalk_packet_type);
-
 	rc = register_netdevice_notifier(&ddp_notifier);
 	if (rc)
 		goto out_snap;
@@ -1998,8 +1911,6 @@  static int __init atalk_init(void)
 out_dev:
 	unregister_netdevice_notifier(&ddp_notifier);
 out_snap:
-	dev_remove_pack(&ppptalk_packet_type);
-	dev_remove_pack(&ltalk_packet_type);
 	unregister_snap_client(ddp_dl);
 out_sock:
 	sock_unregister(PF_APPLETALK);
@@ -2026,8 +1937,6 @@  static void __exit atalk_exit(void)
 	atalk_proc_exit();
 	aarp_cleanup_module();	/* General aarp clean-up. */
 	unregister_netdevice_notifier(&ddp_notifier);
-	dev_remove_pack(&ltalk_packet_type);
-	dev_remove_pack(&ppptalk_packet_type);
 	unregister_snap_client(ddp_dl);
 	sock_unregister(PF_APPLETALK);
 	proto_unregister(&ddp_proto);
diff --git a/net/appletalk/dev.c b/net/appletalk/dev.c
deleted file mode 100644
index 284c8e585533a..0000000000000
--- a/net/appletalk/dev.c
+++ /dev/null
@@ -1,46 +0,0 @@ 
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Moved here from drivers/net/net_init.c, which is:
- *	Written 1993,1994,1995 by Donald Becker.
- */
-
-#include <linux/errno.h>
-#include <linux/module.h>
-#include <linux/netdevice.h>
-#include <linux/if_arp.h>
-#include <linux/if_ltalk.h>
-
-static void ltalk_setup(struct net_device *dev)
-{
-	/* Fill in the fields of the device structure with localtalk-generic values. */
-
-	dev->type		= ARPHRD_LOCALTLK;
-	dev->hard_header_len 	= LTALK_HLEN;
-	dev->mtu		= LTALK_MTU;
-	dev->addr_len		= LTALK_ALEN;
-	dev->tx_queue_len	= 10;
-
-	dev->broadcast[0]	= 0xFF;
-
-	dev->flags		= IFF_BROADCAST|IFF_MULTICAST|IFF_NOARP;
-}
-
-/**
- * alloc_ltalkdev - Allocates and sets up an localtalk device
- * @sizeof_priv: Size of additional driver-private structure to be allocated
- *	for this localtalk device
- *
- * Fill in the fields of the device structure with localtalk-generic
- * values. Basically does everything except registering the device.
- *
- * Constructs a new net device, complete with a private data area of
- * size @sizeof_priv.  A 32-byte (not bit) alignment is enforced for
- * this private data area.
- */
-
-struct net_device *alloc_ltalkdev(int sizeof_priv)
-{
-	return alloc_netdev(sizeof_priv, "lt%d", NET_NAME_UNKNOWN,
-			    ltalk_setup);
-}
-EXPORT_SYMBOL(alloc_ltalkdev);