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

[Joe Hershberger]

On Mon, Apr 27, 2015 at 2:53 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Joe,
>
> On 27 April 2015 at 12:20, Joe Hershberger <joe.hershberger at gmail.com> wrote:
>> Hi Simon,
>>
>> On Thu, Apr 23, 2015 at 11:32 PM, Simon Glass <sjg at chromium.org> wrote:
>>> Hi Joe,
>>>
>>> On 21 April 2015 at 16:02, Joe Hershberger <joe.hershberger at 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?

Perhaps I was unclear... Enabling CONFIG_REGEX for a target used to
add 3550 bytes total (it was already used in env grep command and
setexpr sub/gsub). So that means the code for using that lib on the
env_attr names is only 83 bytes.

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

I agree.

>>
>> 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