[U-Boot] [PATCH] eth_receive(): Do not assume that caller always wants full packet.

[Marcel Moolenaar]

On Jul 16, 2009, at 8:39 AM, Wolfgang Denk wrote:
>> Now, one approach would be to ignore packets that don't
>> fit the buffer. This seems unflexible, because it makes
>> it impossible to employ flexible buffer allocation in
>> the application. What's left? The only thing left is that
>
> "flexible buffer allocation"? May I ask what sort of applications you
> have in mind? We're talking about applications running in the context
> of a boot loader, right? For anything fancy you should probably rather
> use an operating system.

Nothing particular. It was just a random thought that
popped in my head while I was typing. I very much doubt
that anything will be implemented, but it does illustrate
(at least to me :-) how one approach makes assumptions
and thereby eliminating a class of implementations (this
being the "ignore packet" behaviour) while the other
does not, allows that same class of applications, but
does so at the "cost" of having applications check more.

Since we are talking about raw packets, the application
(in this case a boot loader) must check anyway, so the
cost is nihil.

>> you return whatever arrived on the interface truncated
>> to the buffer size. That way the application can discard
>> and call read again if headers don't match, or it can
>> allocate a bigger buffer and retry.
>
> Allocate a bigger buffer? What for? The packet has been dropped
> already and is not recoverable.

The scenario in question is when the truncated packet is
passed to the application. There's no dropping. Depending
on the application it surely can wait for the retransmit
or it can ask for one. Both cases (wait or ask) can be
considered retries...

>> The question is not of effort -- there's virtually none.
>> The question is whether this is good engineering. Worst
>> case buffer allocation doesn't strike me as portable nor
>> reasonable.
>
> Not portable? In which way do you see any porting  issues  here?  Not
> reasonable?  How many such buffers do you need, then? All it takes is
> a different size  in  simple  call  to  malloc(),  or  am  I  missing
> something?

Wolfgang,

It seems to me that these questions stem from an assumption
about how applications are written. That is, I always interpret
these questions as an inquiry into the use of an API so as to
argue about how an API is used, rather than how the API should
behave. Personally, I don't think this is the right approach
when discussing an API, because it the API comes with an assumed
use case.

Anyway: I'll answer your question given above beneath the
following...

> Please let's keep in mind: this is a boot loader, and a very  minimal
> network  stack.  It  is  not an OS, and we don't claim to implement a
> TCP/IP stack.

The application in question is a boot loader. One that does not
implement a TCP/IP stack either. I would argue that it's exactly
the application one would expect to run under U-Boot.

As to the porting issues you were asking about: the FreeBSD boot
loader runs in many environments: PC BIOS, EFI, Open Firmware,
U-Boot, ARC (obsolete), etc.
If The U-Boot APIs requires the FreeBSD loader to allocate 9K
buffers just so that it can receive a tiny ARP response, then
it's the only environment to do so. Changing the FreeBSD loader
to work with U-Boot affects the loader in the other environments
as well. This by itself shows that there's a portability issue.

As for the number of buffers: we need 1 for ARP. A small one I
might add. However, BOOTP/DHCP also needs a buffer. That's one
more. TFTP needs a buffer. etc, etc...
The question as to how many buffers we need cannot really be
answered without digressing in "why don't you restructure your
code so that you only need 1". It all depends on how code is
organized, designed, modularized or combined. In the case of
the FreeBSD loader we a few places to change. This does not
include all the non-open features that people have added to the
loader.


Let me try to spin it differently:

The U-Boot APIs were intended and designed to work with any
application. More to the point: they we designed, implemented
and added to U-Boot *because* of the need arising from wanting
to use the FreeBSD loader under U-Boot.
A such, the APIs were designed with the loader in mind and
they were tested with the FreeBSD loader. Besides adding a
U-Boot interface layer to the loader, no loader changes were
made.

Ok, there's bug. Or at least a scenario that wasn't really
thought about or considered. The pivotal application, key
in designing and implementing the APIs, shows that the API
in U-Boot can hang. [The hang being caused by the reception
of a packet that is larger than the buffer the application
provides]. There are 2 simple fixed that would fix the API:
1) just drop the packet. 2) return a truncated packet.

However, I'm currently in a discussion that suggests that
the application should use bigger buffers. That strikes me
as odd, because by intend and design the API was to support
the application without requiring it to use bigger buffers.
Put differently: by requiring the application to use bigger
buffers, the API ipso-facto stops supporting the one app
it was designed to support. Is this a good way to fix an
otherwise minor problem?

To conclude:
I'm happy with an API change, whether it's dropping the
packet or returning a truncated one. Personally I favor
the truncation, because we're dealing with raw packets
and the application is expected to make sure it received
a proper packet to begin with.

Changing the semantics of the API and require all
applications to allocate a bigger buffer to handle this
is not a solution in my opinion.

In any case: the final verdict is with the U-Boot community.
I stated my case and look forward to the resolution so that
I can assess the consequences to me, FreeBSD, as well as my
employer (in reverse order of importance :-)

Cheers,

-- 
Marcel Moolenaar
xcllnt at mac.com