| Message ID | 20190524212719.30694-1-linus.walleij@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Christian Lamparter |
| Headers | show |
| Series | [OpenWrt-Devel,1/3] gemini: Make a per-board case for ethernet MAC | expand |
Hello, I have a few suggestions inline below. On Friday, May 24, 2019 11:27:17 PM CEST Linus Walleij wrote: > The DNS-313 isn't the only special board so let's bite the > bullet and create a case ladder in preparation for DIR-685. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > .../lib/preinit/05_set_ether_mac_gemini | 30 ++++++++++++------- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > index 1ce5c8067ef0..ebd3ae0f55c5 100644 > --- a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > +++ b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini > @@ -1,6 +1,25 @@ > #!/bin/sh > > set_ether_mac() { > + > + . /lib/functions.sh > + > + case $(board_name) in > + dlink,dns-313) > + # The DNS-313 has a special field in its RedBoot > + # binary that we need to check > + CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" > + if [ ! -z $CONFIG_PARTITION ] ; then This looks familiar. From afar, this is almost like package/base-files/files/lib/functions.sh's find_mtd_part with an extra check. > + DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 2>/dev/null)" I think find_mtd_part's result from above would be the perfect input for dd if=... > + if [ "x$DEVID" = "xdns-313" ] ; then > + MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 count=6 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" Isn't this like package/base-files/files/lib/functions/system.sh's mtd_get_mac_binary function? What does . /lib/functions.sh . /lib/functions/system.sh echo $(mtd_get_mac_binary RedBoot 119540) produce? it should be the same MAC. if it is you could use the get_mac_binary with the find_mtd_part from above and get the mac this way. > + ifconfig eth0 hw ether $MAC1 2>/dev/null I guess while we are at it, why not change it to "ip link set dev eth0 address $MAC1" in case the ifconfig deprecation ever materializes. > + return 0 > + fi > + fi > + ;; > + esac > + > # Most devices have a standard "VCTL" partition > CONFIG_PARTITION="$(grep "VCTL" /proc/mtd | cut -d: -f1)" > if [ ! -z $CONFIG_PARTITION ] ; then > @@ -12,17 +31,6 @@ set_ether_mac() { > return 0 > fi > > - # The DNS-313 has a special field in its RedBoot > - # binary that we need to check > - CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" > - if [ ! -z $CONFIG_PARTITION ] ; then > - DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 2>/dev/null)" > - if [ "x$DEVID" = "xdns-313" ] ; then > - MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 count=6 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" > - ifconfig eth0 hw ether $MAC1 2>/dev/null > - return 0 > - fi > - fi > } > > boot_hook_add preinit_main set_ether_mac >
Hi Christian, I worked in all the changes you requested until I got to this: On Thu, May 30, 2019 at 12:46 AM Christian Lamparter <chunkeey@gmail.com> wrote: > > + ifconfig eth0 hw ether $MAC1 2>/dev/null > > I guess while we are at it, why not change it to > "ip link set dev eth0 address $MAC1" Testing this: ip link set eth0 address 00:50:c2:11:11:11 ip: socket(AF_PACKET,2,0): Address family not supported by protocol Any hints? According to the help it should work ... Yours, Linus Walleij
Hello Linus, On Tuesday, June 11, 2019 4:40:12 PM CEST Linus Walleij wrote: > Hi Christian, > > I worked in all the changes you requested until I got to this: > > On Thu, May 30, 2019 at 12:46 AM Christian Lamparter <chunkeey@gmail.com> wrote: > > > > + ifconfig eth0 hw ether $MAC1 2>/dev/null > > > > I guess while we are at it, why not change it to > > "ip link set dev eth0 address $MAC1" > > Testing this: > > ip link set eth0 address 00:50:c2:11:11:11 > ip: socket(AF_PACKET,2,0): Address family not supported by protocol > > Any hints? According to the help it should work ... Yeah, now that I'm home (I'm away pretty much "away" during the workdays of the week) I did take a quick look. target/linux/gemini/config-4.14 and target/linux/gemini/config-4.19 both have # CONFIG_PACKET is not set I think these should be removed (so CONFIG_PACKET is enabled because target/linux/generic/config-4.1X sets it to =y)? Because otherwise other helpful tools like tcpdump won't work either. I put together a patchset in my staging tree starting from https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=620462e09afc40602110b82caeb2858903709567 https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=bbde89504fb6124da5a3f0014025753db4d6ec67 https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=6096e5208373822d123b8fe4f848b8612f3d04c8 https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=1d8ddcfc7a00701ab73d7dd06cf5fa420c1a5882 https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=b106a522001e970378c38279fe598acbc867d0f4 Let me know if this works now with the ip tool. Regards, Christian
On Fri, Jun 14, 2019 at 7:24 PM Christian Lamparter <chunkeey@gmail.com> wrote: > I put together a patchset in my staging tree starting from > > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=620462e09afc40602110b82caeb2858903709567 > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=bbde89504fb6124da5a3f0014025753db4d6ec67 > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=6096e5208373822d123b8fe4f848b8612f3d04c8 > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=1d8ddcfc7a00701ab73d7dd06cf5fa420c1a5882 > https://git.openwrt.org/?p=openwrt/staging/chunkeey.git;a=commit;h=b106a522001e970378c38279fe598acbc867d0f4 > > Let me know if this works now with the ip tool. Yes! I applied this series and all comes up nicely, right MAC address and writeable root filesystem. Feel free to push this and thanks so much for the help! Yours, Linus Walleij
diff --git a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini index 1ce5c8067ef0..ebd3ae0f55c5 100644 --- a/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini +++ b/target/linux/gemini/base-files/lib/preinit/05_set_ether_mac_gemini @@ -1,6 +1,25 @@ #!/bin/sh set_ether_mac() { + + . /lib/functions.sh + + case $(board_name) in + dlink,dns-313) + # The DNS-313 has a special field in its RedBoot + # binary that we need to check + CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" + if [ ! -z $CONFIG_PARTITION ] ; then + DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 2>/dev/null)" + if [ "x$DEVID" = "xdns-313" ] ; then + MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 count=6 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" + ifconfig eth0 hw ether $MAC1 2>/dev/null + return 0 + fi + fi + ;; + esac + # Most devices have a standard "VCTL" partition CONFIG_PARTITION="$(grep "VCTL" /proc/mtd | cut -d: -f1)" if [ ! -z $CONFIG_PARTITION ] ; then @@ -12,17 +31,6 @@ set_ether_mac() { return 0 fi - # The DNS-313 has a special field in its RedBoot - # binary that we need to check - CONFIG_PARTITION="$(grep "RedBoot" /proc/mtd | cut -d: -f1)" - if [ ! -z $CONFIG_PARTITION ] ; then - DEVID="$(dd if=/dev/mtdblock0 bs=1 skip=119508 count=7 2>/dev/null)" - if [ "x$DEVID" = "xdns-313" ] ; then - MAC1="$(dd if=/dev/mtdblock0 bs=1 skip=119540 count=6 2>/dev/null | hexdump -n6 -e '/1 ":%02X"' | sed s/^://g)" - ifconfig eth0 hw ether $MAC1 2>/dev/null - return 0 - fi - fi } boot_hook_add preinit_main set_ether_mac
The DNS-313 isn't the only special board so let's bite the bullet and create a case ladder in preparation for DIR-685. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- .../lib/preinit/05_set_ether_mac_gemini | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-)