[U-Boot] [PATCH 00/11] Improve env var handling for net stack

Simon Glass
2015-04-27T19:53:39Z

Hi Joe,

On 27 April 2015 at 12:20, Joe Hershberger <joe.hershberger@gmail.com> wrote:
> Hi Simon,
>
> On Thu, Apr 23, 2015 at 11:32 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Joe,
>>
>> On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger@ni.com> wrote:
>>> This includes moving CONFIG_REGEX to Kconfig and adding support for
>>> regex to the env_attr lists (when CONFIG_REGEX is enabled).
>>>
>>> This allows ethaddrs to all be checked for access and format by default.
>>> Also use callbacks to keep network stack variables up to date instead of
>>> polling them on each call to net_loop.
>>>
>>> This is a step in the right direction to refactoring the network stack
>>> to be similar to that of barebox.
>>>
>>>
>>> Joe Hershberger (11):
>>> sandbox: Enable some ENV commands
>>> kconfig: Move REGEX to Kconfig
>>> sandbox: Enable regex support
>>> env: Fix return values in env_attr_lookup()
>>> env: Simplify the reverse_strstr() interface
>>> env: Allow env_attr_walk to pass a priv * to callback
>>> env: Add regex support to env_attrs
>>> env: Distinguish finer between source of env change
>>> net: Apply default format rules to all ethaddr
>>> net: Use env callbacks for net variables
>>> net: Add default flags for common net env vars
>>>
>>> common/cmd_nvedit.c | 36 +++++---
>>> common/env_attr.c | 179 ++++++++++++++++++++++++++++---------
>>> common/env_callback.c | 6 +-
>>> common/env_flags.c | 6 +-
>>> configs/acadia_defconfig | 1 +
>>> configs/bamboo_defconfig | 1 +
>>> configs/bubinga_defconfig | 1 +
>>> configs/canyonlands_defconfig | 1 +
>>> configs/dlvision-10g_defconfig | 1 +
>>> configs/dlvision_defconfig | 1 +
>>> configs/ebony_defconfig | 1 +
>>> configs/gdppc440etx_defconfig | 1 +
>>> configs/icon_defconfig | 1 +
>>> configs/intip_defconfig | 1 +
>>> configs/io64_defconfig | 1 +
>>> configs/io_defconfig | 1 +
>>> configs/iocon_defconfig | 1 +
>>> configs/katmai_defconfig | 1 +
>>> configs/kilauea_defconfig | 1 +
>>> configs/luan_defconfig | 1 +
>>> configs/m28evk_defconfig | 1 +
>>> configs/m53evk_defconfig | 1 +
>>> configs/makalu_defconfig | 1 +
>>> configs/neo_defconfig | 1 +
>>> configs/novena_defconfig | 1 +
>>> configs/ocotea_defconfig | 1 +
>>> configs/redwood_defconfig | 1 +
>>> configs/sandbox_defconfig | 1 +
>>> configs/sequoia_defconfig | 1 +
>>> configs/socfpga_arria5_defconfig | 1 +
>>> configs/socfpga_cyclone5_defconfig | 1 +
>>> configs/t3corp_defconfig | 1 +
>>> configs/taihu_defconfig | 1 +
>>> configs/taishan_defconfig | 1 +
>>> configs/walnut_defconfig | 1 +
>>> configs/yosemite_defconfig | 1 +
>>> configs/yucca_defconfig | 1 +
>>> include/configs/amcc-common.h | 1 -
>>> include/configs/m28evk.h | 1 -
>>> include/configs/m53evk.h | 1 -
>>> include/configs/novena.h | 1 -
>>> include/configs/sandbox.h | 5 ++
>>> include/configs/socfpga_arria5.h | 1 -
>>> include/configs/socfpga_cyclone5.h | 1 -
>>> include/env_attr.h | 10 +--
>>> include/env_callback.h | 32 ++++++-
>>> include/env_flags.h | 23 ++++-
>>> include/search.h | 2 +
>>> lib/Kconfig | 8 ++
>>> net/net.c | 105 ++++++++++++++++++----
>>> test/dm/eth.c | 1 +
>>> 51 files changed, 358 insertions(+), 94 deletions(-)
>>
>> Looks good! I wonder if you could update a README somewhere to explain
>> how it works?
>
> I'll update README to describe it.
>
>> If I understand correctly, you need to enable CONFIG_REGEX for the
>> eth1addr variable to work (for example). Is that right? If so, what is
>> the code size impact?
>
> That's sort-of correct. Before the regex, only the "ethaddr" was
> checked for format, though the eth1addr, etc. all "worked", just were
> unverified.
>
> I did some build tests...
>
> Without CONFIG_REGEX, the total size grew by 295 bytes (due to the
> other global changes).
> With CONFIG_REGEX enabled, the total size grew by 378 bytes.
> Enabling CONFIG_REGEX now adds 3633 bytes total.

Probably because it is now actually being used?

This is definitely a size increase but IMO it is worth it and those
using networking are likely less size-sensitive.

>
> This test was on BB Black (ARM).
>
> Raw data:
>
> text data bss dec hex filename
> W/ patch W/O regex
> 394014 13324 305876 713214 ae1fe /tmp/u-boot-build/arm/u-boot
> W/ patch W/ regex
> 397651 13324 305872 716847 af02f /tmp/u-boot-build/arm/u-boot
> W/O patch W/O regex
> 393811 13276 305832 712919 ae0d7 /tmp/u-boot-build/arm/u-boot
> W/O patch W/ regex
> 397333 13276 305860 716469 aeeb5 /tmp/u-boot-build/arm/u-boot
>
> Cheers,
> -Joe

Regards,
Simon