driver chokes if bufcnt < 2

[mischief]

if a aoe target sends a bufcnt of 0 or 1, the driver will choke with no indication of the problem.

the issue shows up during handling the config query response. if bufcnt is 1, t->maxout is calculated as 0 at aoecmd.c#L1553. later, newframe is called to send the ata identify command, but the frame is NULL at aoecmd.c#L284 because of the t->nout < t->maxout check.

maybe there should be a warning print in the driver if this occurs. it took me a long time to debug my target implementation because of this small problem :)

[ecashin]

Thanks for the suggestion. I am sick now but hope to look into that soon.

[mischief]

sorry to hear it, take care :-)

[ecashin]

On closer examination, it looks like the driver doesn't become unusable, and the kernel doesn't crash, but the new device is unusable. If undefined behavior occurs when the AoE target is behaving outside the spec, that kind of makes sense, as long as it only affects the misbehaving target.

However, as you discovered, the aoe driver could help make it easier for people to create or modify new targets if it logged a warning.

In the past, the Linux Kernel Mailing List (LKML) developers have been wary of doing that without rate limiting. You can see other parts in the aoe driver where we use net_ratelimit() to avoid saturating the logging system with warnings.

If all that sounds reasonable to you, I'll add such a rate-limited warning. And please let me know if there are other places like this where we could encourage AoE target developers without complicating the code.

[mischief]

I should have been more specific - it only chokes up on the misbehaving
target.

A rate limited message sounds fine.
On Mar 16, 2016 5:35 PM, "Ed Cashin" notifications@github.com wrote:

On closer examination, it looks like the driver doesn't become unusable,
and the kernel doesn't crash, but the new device is unusable. If undefined
behavior occurs when the AoE target is behaving outside the spec, that kind
of makes sense, as long as it only affects the misbehaving target.

However, as you discovered, the aoe driver could help make it easier for
people to create or modify new targets if it logged a warning.

In the past, the Linux Kernel Mailing List (LKML) developers have been
wary of doing that without rate limiting. You can see other parts in the
aoe driver where we use net_ratelimit() to avoid saturating the logging
system with warnings.

If all that sounds reasonable to you, I'll add such a rate-limited
warning. And please let me know if there are other places like this where
we could encourage AoE target developers without complicating the code.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#7 (comment)

[ecashin]

Hi, @mischief. Can you please check that the changes here:

#9

... lead to the desired behavior? Hopefully you can comment on the pull request, but I've only used Stash/Bitbucket web-based pull request discussions before, so I'm not sure.