Hacker News new | past | comments | ask | show | jobs | submit login
Hacking up a fix for the broken AppleTalk kernel module in Linux 5.1 and newer (downtowndougbrown.com)
106 points by zdw on Aug 1, 2020 | hide | past | favorite | 40 comments

"In this case, someone submitted a patch 10 months ago and was asked to tweak the patch. Then life got in the way (I totally understand), and he finally tried again about 9 months later...

He was also asked to tweak the second patch because of a minor style issue (which I also understand). A third try at submitting the patch hasn’t been attempted as of this writing."

I've been there. I understand both sides, but it's a bit frustrating from the "outside contributor" standpoint sometimes.

For example, I use git quite a lot, but not GitHub. As a result, a pretty simple "couple of lines" fix I tried to contribute remains uncommitted because I wasn't able to format things in a way that made the target project folks happy. The pull request remains sitting there. Though I still occasionally get "thank you" messages from people that accidentally find and use it.

So, some number of folks lose out on a good fix roughly because what would take me several hours, but take someone else a few minutes, just doesn't get done.

Some sort of web based "force this minor tweak for a pull request" feature might have a decent upside.

GitHub has a feature for this: https://docs.github.com/en/github/collaborating-with-issues-...

The implementation is, if you enable this option when making a pull request, people with write access to the target project also get write access to your branch, so they can grab it, make some fixes, git commit (perhaps with --amend), git push -f to your fork, and then merge it from your pull request, and you keep authorship and the pull request gets resolved like normal.

For what it's worth, the Linux kernel community has a process for this too - a subsystem maintainer can absolutely grab someone's patch and git commit --amend. Usually it's considered courteous to add a "[maintainer@example.com: fixed style]" line at the bottom of the commit message. I'm a little surprised the maintainer didn't do that in this case but I bet life got in their way too :)

Ahh... that's enlightening and helpful. Reading up now. Thanks for sharing.

When I first started contributing to open source projects on sourceforge using cvs or svn, it was normal to send patches to the mailing list and for the maintainer to say something like "thanks, I've applied it with a few small changes". I feel that the workflow that GitHub pull requests encourages is that a maintainer will often make detailed comments and it's up to the contributor to fix every last detail, which can often be difficult if they're a new contributor.

In some of my projects, when a new contributor submits a first patch, I've started to do something similar to what people did for me in the past. I will work with them to make major improvements if needed, but if there are a few small things (fix some styling, improve a test, etc), I will just make those changes myself in order to get the PR merged. I understand that this isn't always possible on all projects, and depending on people's spare time, but I think that it's a nice way of showing people that their contributions are welcome without getting bogged down in rules about whitespace or naming.

Is there some kind of cultural reason in the process of submitting patches to linux that someone else couldn't come along and, while giving due credit, do the style fixups on one or the other of the patches themselves and submit that as a v3/v2?

Like, why should every attempt start from scratch as people lose interest?

That's exactly what I was mulling over in my head today (I'm the author of this blog post). If there's some way to simply provide a v3 of this patch while leaving the credit with the original author, I'd love to do it. I just didn't want to step on anybody's toes, and I don't know the etiquette. I think this kind of thing happens more often than we realize. I've seen similar situations before on other drivers.

Download the raw patch from lore.kernel.org, apply it with "git am", fix up the commit message with "git commit --amend", add your "Signed-off-by" below the existing one, submit with "git-send-email" or "git format-patch" + "msmtp".

That way the commit author remains the same because "git format-patch" includes a "From:" line in the message body which "git am" will interpret as the commit author when the maintainer applies the patch.

Your Signed-off-by is necessary to keep the DCO chain intact: https://www.kernel.org/doc/html/latest/process/submitting-pa...

However, that is now done: https://lore.kernel.org/netdev/086b426f44bc24360cc89476fe18d...

Thank you for the report.

Thank you! That is very useful to know for the future.

Honestly, fuck the ettiquite. Reubsmit the patch with fixedup styling and attribution to the original author. If it ruffles some feathers ? Who cares. Hopefully it shakes things up.

Yes, that's roughly the jist of my comment. There are often people with the skills to fix stuff that want to contribute back. But, there's typically a line for them. They fixed it for their own purposes, but are only willing to invest so much time in learning and aligning to project wishes and practices.

I get there are opposite examples of contributors just being contrary and rude, but some sort of workflow to quickly make good, but standards-flawed fixes compliant has merit.

Your idea of a simple way to let someone else take over a pull request seems good.

In general, a lot of contribution workflows are fairly daunting to newcomers–when I tried to submit a fairly small patch to the Linux kernel it took me a while before I figured out what to do, and even then I messed it up and it didn't end up going in. (Full disclaimer: I commented about it here and someone was extremely kind in sending me resources on how to actually do this. I haven't yet followed that advice, sorry!) But even with this, it's still really complicated to figure out how each project does things. I understand many don't really care about one-line patches, but it would be nice if this kind of thing (which I personally consider "outreach") was better supported.

Bravo. Sometimes those one-line patches are gold. I still use a one-line patch for a popular Perl serial/rs-232 module that some kind soul tried to contribute, but failed to get accepted. It's been that way for 10 years or so.

I have heard the other side, which is that many one-line patches require significant time from maintainers because it's often "hand-holding" to get them to where they need to be and often they are not really correct or have some other issues. But even with that I think the case of "maintainer abandoned this patch" or "maintainer is actively looking for new contributors but does not make it easy to get your foot in the door" is the more common one.

When submitting a pull request upstream on GitHub there’s the option (I believe enabled by default) to allow members of the upstream repository to push to your branch, presumably for just this reason of it needing a few tweaks.

In cases like this where a new contributor has a patch with a minor style issue, the code reviewer should probably just accept the patch and repeat it themselves. Or have a group of intermediate contributors who can clean up or shepherd patches across the finish line.

That feels like a pretty easy "shephard" feature that GitHub and/or GitLab could implement.

I don't think there's generally a cultural reason. It just doesn't happen all that often because the "someone else" coming along probably isn't familiar with the project workflow either so they don't know the right way to do it.

What does happen more often I think is that one of the maintainers or regular devs will make minor fixups as part of accepting the patch. (I do this for the project I work on for new contributions at least some of the time.) But that does take time, and usually the people working regularly on the project have a full todo list of stuff they care about or are paid to care about already. So if the patch is for an obscure and unloved area of the codebase (like, say, support for a networking protocol used only by ancient Macs) it's more likely to fall through the cracks simply because nobody working regularly on the project cares about that area. Niche use-cases which have users but no developers are fundamentally in trouble long-term, I think.

This is a case study in why people get discouraged from contributing to open source. When I maintained a linux driver, I absolutely dreaded dealing with the netdev mailing list. I used to delegate that to linux fanatics in the company when I could.

> Though I still occasionally get "thank you" messages from people that accidentally find and use it.

Why not ask one of them to make a pull request using the proper formatting?

Edit: clarification

Honestly? Apathy and Laziness.

Github actually has a "Allow maintainers to modify this branch" option.

If you set this, you could just ask the maintainers to push their changes to your PR and be done with it

If you want this fixed forever, a regression test needs to be created for it. I have no idea how this works for the Linux kernel, but Google search reveals this project:


That was a really fun debugging ride. I've never really had to debug kernel stuff before, so I also learned some things. Then you topped it off with some serious hacks. You sure know the way into my heart. Thanks for writing about it!

While I don't have a personal use for AppleTalk support, it makes me happy that support for old protocols lives on.

Ah, stripping the signature from a binary you have modified. Glad to hear that others get to enjoy this previously mostly macOS-unique diversion now ;)

The problem here is that Ubuntu’s kernel modules are signed.

It's easy to bypass, as the article shows, but also sad that this user-hostile practice has spread to Linux. I'd expect this for Windows or Mac, but not Linux.

It's not a user hostile practice if you have control over it.

Do you consider signing keys in apt packages to also be user hostile?

Is there an advantage to enable AFP on my NAS?

I have a Macbook but smb seems kinda shady there.

Newer macOS prefers SMB3 over AFP, even for time machine backups. There’s no reason to use AFP anymore on a modern Mac.

Except that AFP is 3 to 5 times faster on 10GigE. SMB implementation on MacOS has always been a joke, and still is. As long as you're only networking with WiFi, it's probably OK. But when you do serious video editing, AFP still reigns supreme (but apparently almost nobody knows it).

Case in point : recently, tested using a Hackintosh and a hand-built high performance NAS (100 TB), 10GigE ethernet.

Running Windows 10 on the Hackintosh, SMB protocol : ~900 MB/s read/write

Running MacOS on the same machine , SMB protocol : ~200 MB/s read/write

Running MacOS, AFP protocol : ~1 GB/s read/write.

And you don't even need the AppleTalk module for AFP. (I don't think modern macOS even supports AppleTalk anymore.) AppleTalk only comes into play when you're dealing with really old Mac systems which didn't support AFP-over-TCP yet.

What hardware do you use to connect the two?

All the things described are running over ethernet, there isn't any special hardware.

The kernel module implements the stuff described in the sections on EtherTalk and AppleTalk phase II on this [1] wiki page.

[1] https://en.wikipedia.org/wiki/AppleTalk

So just a LocalTalk to Ethernet bridge?

No, the old Macintosh uses its ethernet controller to connect to the Linux server, LocalTalk isn't used.

I see.

When the article mentioned AppleTalk I envisioned a way to get files off of my old SE using something that hooked into the ADB networking ports.

That too is possible (serial ports though, not ADB). There are existing LocalTalk-to-Ethernet bridge products from long ago that are getting harder and harder to find. I was actually working on a new one for a while but ran out of the time/motivation to finish it up. https://mac68k.info/forums/thread.jspa?threadID=275&start=0&...

Great info, thanks

You wouldn't use an ADB port, LocalTalk used the printer port.

For your SE you could get an ethernet adapter that plugged into the SCSI port or a PDS card to go inside the case.

AppleTalk works fine on NetBSD. Doesn't require some long process to fix anything that breaks.

My point was that maybe the Linux development process isn't a good match for niche areas like this. With NetBSD+pkgsrc, I can (and have done) just commit fixes to the kernel and userland AppleTalk code directly without needing to go through gatekeepers who may not have used this feature.

The blog author might want to look at a recent pkgsrc patch to netatalk 2.2 that makes it work with current CUPS.

Applications are open for YC Winter 2023

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact