RasppleII / a2server

AppleTalk server for Apple II computers
Other
31 stars 8 forks source link

Don't add eth0 to atalkd.conf if there's already an interface defined #47

Closed IvanExpert closed 8 years ago

IvanExpert commented 8 years ago

a2server-setup currently adds an entry for eth0 in atalkd.conf if it doesn't find one, even if another entry exists for a different interface. It should stop doing this.

knghtbrd commented 8 years ago

On Sun, Nov 22, 2015 at 05:18:13PM -0800, IvanExpert wrote:

a2server-setup currently adds an entry for eth0 in atalkd.conf if it doesn't find one, even if another entry exists for a different interface. It should stop doing this.

I find this in a2server-3-sharing.txt:

if [[ ! $(grep ^eth0 /usr/local/etc/netatalk/atalkd.conf) && ! $(grep ^wlan0 /usr/local/etc/netatalk/atalkd.conf) ]]; then
    # enable netatalk on the default network interface
    # needs -router and -zone to prevent GS/OS AppleShare CDEV crash when used
    # with Dayna or Asante bridges
    echo -e 'eth0 -router -phase 2 -net 1 -zone "A2SERVER"' \
        | sudo tee -a /usr/local/etc/netatalk/atalkd.conf > /dev/null
fi

It seems like the solution here is to

  1. Strip out any comment line
  2. Strip any whitespace lines
  3. Determine how many lines remain. If more than one, add it.

Does that seem like an appropriate fix?

knghtbrd commented 8 years ago

FWIW I was thinking something along this line:

sed -e 's/#.*$//g' -e '/^\s*$/d'
IvanExpert commented 8 years ago

Hmm. Wouldn't that remove everything in the file, which otherwise provides lots of documentation in its comments for users wanting to configure it? (And I don't understand step three of your logic, above.) I'm generally of the school of touch-as-little-as-possible.

I was thinking about something along the lines of keeping the logic as is, but have smarter detection; look at the end of the file, and (going backwards) if there's a comment before a non-comment, write out the default interface; and if there is a non-comment, then rather than hardcoding for eth0 or wlan0, just look for "-phase 2" and if that's in there, then assume the interface already exists, otherwise it should create the eth0 default. (Or, if we really want to get fancy, capture the interface name on the "-phase 2" line, and compare it against existing interfaces as reported by ifconfig, and write the eth0 default if none match.)

knghtbrd commented 8 years ago

On Sun, Nov 22, 2015 at 07:44:16PM -0800, IvanExpert wrote:

Hmm. Wouldn't that remove everything in the file, which otherwise provides lots of documentation in its comments for users wanting to configure it? (And I don't understand step three of your logic, above.) I'm generally of the school of touch-as-little-as-possible.

That was for reading, not for writing. That output would be one or more lines of configuration with nothing else to be fed into wc -l.
If wc says there are no lines, then you have no configuration and need us to write a default. If it says 1 or more, then we shouldn't touch your configuration.

I was thinking about something along the lines of keeping the logic as is, but have smarter detection; look at the end of the file, and (going backwards) if there's a comment before a non-comment, write out the default interface; and if there is a non-comment, then rather than hardcoding for eth0 or wlan0, just look for "-phase 2" and if that's in there, then assume the interface already exists, otherwise it should create the eth0 default. (Or, if we really want to get fancy, capture the interface name on the "-phase 2" line, and compare it against existing interfaces as reported by ifconfig, and write the eth0 default if none match.)

That can be done too. Maybe search just the last few lines instead of the whole file? Whichever you think is best.

IvanExpert commented 8 years ago

We could search the last few lines, but to eliminate any guesswork why not just run it through 'tac', and consider only the lines before the first comment (if there are any)?

On Nov 23, 2015, at 12:11 AM, Joseph Carter notifications@github.com wrote:

On Sun, Nov 22, 2015 at 07:44:16PM -0800, IvanExpert wrote:

Hmm. Wouldn't that remove everything in the file, which otherwise provides lots of documentation in its comments for users wanting to configure it? (And I don't understand step three of your logic, above.) I'm generally of the school of touch-as-little-as-possible.

That was for reading, not for writing. That output would be one or more lines of configuration with nothing else to be fed into wc -l. If wc says there are no lines, then you have no configuration and need us to write a default. If it says 1 or more, then we shouldn't touch your configuration.

I was thinking about something along the lines of keeping the logic as is, but have smarter detection; look at the end of the file, and (going backwards) if there's a comment before a non-comment, write out the default interface; and if there is a non-comment, then rather than hardcoding for eth0 or wlan0, just look for "-phase 2" and if that's in there, then assume the interface already exists, otherwise it should create the eth0 default. (Or, if we really want to get fancy, capture the interface name on the "-phase 2" line, and compare it against existing interfaces as reported by ifconfig, and write the eth0 default if none match.)

That can be done too. Maybe search just the last few lines instead of the whole file? Whichever you think is best. — Reply to this email directly or view it on GitHub https://github.com/RasppleII/a2server/issues/47#issuecomment-158854825.

knghtbrd commented 8 years ago

I can imagine there being a configured line followed by a disabled configuration. I have commented lines at the end of my /boot/config.txt for example.

knghtbrd commented 8 years ago

netstat -i shows you the current configured interfaces.

The ugly but effective sed -e '1,2d' -e '/^lo /d' -e 's/ .*$//' will delete the two header lines, delete the loopback interface, and return just the first word of each line (so the interface). If that returns just eth0, then you have no wifi interfaces. If it returns eth0 and wlan0, you have both. If it returns things that don't start with eth or wlan, you have a more interesting network setup. We could possibly replace the first two expressions with -e '/^eth\|wlan/!d' if we want to select only real ethernet and wifi interfaces and avoid possibly matching PPP devices, virtual network devices, etc.

Not real sure this will be useful for resolving this issue or not, but it's a useful way of getting a list of choices.

IvanExpert commented 8 years ago

I implemented this in 1.5.0 by simply checking to see whether the first non-blank line at the end of the script starts with a # or not. If not, it leaves it alone; if does, eth0 is added as an interface as usual.