freifunk-franken
/
firmware
9
7
Fork 13
Code Issues 48 Pull Requests 7 Releases 17 Wiki Activity

treewide/fff-network: use upstream functions for IP concatenation and MAC bit flipping #77

Closed
adschm wants to merge 1 commits from adschm/firmware:owip into master
pull from: adschm/firmware:owip
merge into: freifunk-franken:master
Conversation 2 Commits 1 Files Changed 11 +19 -30
adschm commented 2021-01-13 21:35:20 +01:00
Owner
Copy Link

This proposes two patches that address individual conversions of local IP/MAC address stuff to upstream functions/tools.

This is completely untested.

This proposes two patches that address individual conversions of local IP/MAC address stuff to upstream functions/tools. This is completely untested.
adschm added the
packages/fff
 label 2021-01-13 21:35:20 +01:00
fbl reviewed 2021-01-14 00:11:31 +01:00
src/packages/fff/fff-hoods/files/usr/sbin/configurehood Outdated
@ -198,0 +192,4 @@
# remove escape character
prefix=$(echo "$prefix" | sed -e 's,\\,,')
# create proper prefix
prefix=$(echo "$prefix" | sed -e 's,/,:/,')
fbl commented 2021-01-14 00:11:31 +01:00
Owner
Copy Link

Das hier muss auf jeden Fall noch so geändert werden, dass es korrekt formatierte Prefixe ebenfalls richtig verarbeiten kann. Deswegen haben wir das ja überhaupt erst angefasst.

Hier könnte das aus #69 übernommen werden:

prefix="$(echo "$prefix" | sed -e 's,\([^:]\):\/,\1::/,')"
Das hier muss auf jeden Fall noch so geändert werden, dass es korrekt formatierte Prefixe ebenfalls richtig verarbeiten kann. Deswegen haben wir das ja überhaupt erst angefasst. Hier könnte das aus #69 übernommen werden: ``` prefix="$(echo "$prefix" | sed -e 's,\([^:]\):\/,\1::/,')" ```
adschm commented 2021-01-14 01:11:51 +01:00
Author
Owner
Copy Link

Sicherlich, ich wollte das nur in separaten Patches haben, sodass mein Patch zunächst pseudo-kosmetisch ist.

Sicherlich, ich wollte das nur in separaten Patches haben, sodass mein Patch zunächst pseudo-kosmetisch ist.
adschm commented 2021-01-22 11:37:54 +01:00
Author
Owner
Copy Link

Run-Tested with node on TL-WR841 v12.

owipcalc baut und macht richtige Adressen. Den local-bit Patch habe ich nicht explizit getestet, allerdings sehe ich da ohnehin keinen echten Bedarf.

Run-Tested with node on TL-WR841 v12. owipcalc baut und macht richtige Adressen. Den local-bit Patch habe ich nicht explizit getestet, allerdings sehe ich da ohnehin keinen echten Bedarf.
fbl approved these changes 2021-01-27 12:24:28 +01:00
fbl left a comment
Owner
Copy Link

Von der Änderung auf macaddr_setbit_la halte ich aktuell nicht so viel, da es durchaus auch den Fall geben könnte, dass wir von der local auf die global Adresse wechseln müssen. Da die beiden Commits aber unabhängig voneinander sind, können wir das ja auch einfach erst mal weglassen.

Die Geschichte mit owipcalc sieht gut aus, gefällt mir.
Ich werde noch den entsprechenden Patch nachreichen, mit dem im Hoodfile dann auch korrekt formatierte Prefixe korrekt verarbeitet werden.

Abgesehen von der kommentierten Kleinigkeit:
Reviewed-by: Fabian Bläse <fabian@blaese.de>

Von der Änderung auf `macaddr_setbit_la` halte ich aktuell nicht so viel, da es durchaus auch den Fall geben könnte, dass wir von der local auf die global Adresse wechseln müssen. Da die beiden Commits aber unabhängig voneinander sind, können wir das ja auch einfach erst mal weglassen. Die Geschichte mit owipcalc sieht gut aus, gefällt mir. Ich werde noch den entsprechenden Patch nachreichen, mit dem im Hoodfile dann auch korrekt formatierte Prefixe korrekt verarbeitet werden. Abgesehen von der kommentierten Kleinigkeit: `Reviewed-by: Fabian Bläse <fabian@blaese.de>`
src/packages/fff/fff-network/files/usr/sbin/configurenetwork Outdated
@ -199,2 +198,3 @@
prefix="fdff::/64"
# Set $prefix::MAC as IP
addr="$(ipMacAssemble "$prefix" "$ROUTERMAC")"
addr=$(owipcalc "$prefix" add "::$(ipMacSuffix "$ROUTERMAC")")
fbl commented 2021-01-27 12:20:51 +01:00
Owner
Copy Link

Das '::' gehört zum Suffix. Da wir es eh immer brauchen, würde ich es von ipMacSuffix() anhängen lassen.

Das '::' gehört zum Suffix. Da wir es eh immer brauchen, würde ich es von ipMacSuffix() anhängen lassen.
adschm commented 2021-01-27 18:59:08 +01:00
Author
Owner
Copy Link

Die Frage ist hier, was ein Suffix ist. Ich sehe das Suffix nur als die letzten 64-byte an, also im Prinzip ein EUI-64. Dadurch ist die Funktion auch für andere Zwecke flexibler einsetzbar.

Die Frage ist hier, was ein Suffix ist. Ich sehe das Suffix nur als die letzten 64-byte an, also im Prinzip ein EUI-64. Dadurch ist die Funktion auch für andere Zwecke flexibler einsetzbar.
fbl added this to the 20210211-beta milestone 2021-01-27 12:25:28 +01:00
adschm commented 2021-01-27 19:45:49 +01:00
Author
Owner
Copy Link

Ich werde der Einfachheit halber mal den owipcalc Patch ohne die vorgeschlagene Änderung mergen (ohne ist das ja eine separate Änderung der relevanten Funktion). Ist wahrscheinlich so auch einfacher, dann kann man parallel schon mal die Sache mit dem Präfix anfassen.

Ich werde der Einfachheit halber mal den owipcalc Patch ohne die vorgeschlagene Änderung mergen (ohne ist das ja eine separate Änderung der relevanten Funktion). Ist wahrscheinlich so auch einfacher, dann kann man parallel schon mal die Sache mit dem Präfix anfassen.
adschm force-pushed owip from 32d7ee6251 to 49c2ee11d6 2021-01-27 20:16:34 +01:00 Compare
fbl removed this from the 20210211-beta milestone 2021-01-30 23:31:35 +01:00
adschm force-pushed owip from 49c2ee11d6 to 70873b58fe 2021-02-09 23:10:04 +01:00 Compare
adschm closed this pull request 2021-08-24 02:10:28 +02:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
fbl
Labels
Clear labels   
RFC

   
RFT

   
WIP

   
blocked

   
bsp

touching bsp files or adding devices

  
bug

Something is not working

  
build/scripts/tools

touching buildscript or other tools

  
duplicate

This issue or pull request already exists

  
feature

New feature

  
fixed

Issue that got resolved

  
layer3

only for layer3 FW

  
mantis

Imported from mantis

  
more details required

Reported information is insufficient to debug the issue

  
needs changes

   
node

only for node FW

  
packages/fff

touching our packages

  
rejected

Rejected or won't fix

  
security

   
trivial

simple non-functional changes that are mostly matter of taste

  
upstream

touching upstream version, build or feed patches

No Label
RFC
RFT
WIP
blocked
bsp
bug
build/scripts/tools
duplicate
feature
fixed
layer3
mantis
more details required
needs changes
node
packages/fff
rejected
security
trivial
upstream
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: freifunk-franken/firmware#77
No description provided.
Delete Branch "adschm/firmware:owip"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?