2010-06-11 20:33:52

by David VomLehn

[permalink] [raw]
Subject: [PATCH 1/4][RFC] Introduction to sk_buff state checking

Add state checking for sk_buffs

Several common errors in sk_buff usage can be caught by maintaining and
checking the sk_buff state. States are: free, allocated, and queued. Simply
reporting an error is not sufficient; it may be the previous state change
that was erroneous. Thus, the ability to record the location of the previous
state change is essential.

This patch was previously submitted and David Miller objected to to the
overhead of recording the previous state change. Specifically, he wrote:

The specifics are that if you showed me something that
added a "u16", at most, I'd go to bat for you and support
your work going upstream.

With only three states, only two bits are required to record the current
state but recording the location of the call that changed the state in
no more than 14 bits posed an interesting challenge. This patchset contains
the infrastructure required to map between a small value, the "callsite ID",
and the location of the call.

There are four patches in this patchset:
1. This introduction
2. A patch allowing the site of a call to be passed over multiple calls
without modifying the intervening functions.
3. A patch providing infrastructure that allows the location of a call,
the "callsite ID" to be encoded in a small integer.
4. Modifications to the sk_buff functions to add the two-bit state
and a 14-bit callsite ID, check the state, and report failures.

Restrictions
o Call site IDs are never reused, so it is possible to exceed the
maximum number of IDs by having a large number of call locations.
In addition, it does not recognize that the same module has been
unloaded and reloaded, so calls from the reloaded module will be
assigned new IDs. Detection of incorrect operations on an sk_buff
is not affected by exhaustion of call site IDs, but it may not be
possible to determine the location of the last operation.
CONFIG_DEBUG_SKB_ID_SIZE is set to reduce the sk_buff growth to 16
bits and should handle most cases. It could be made larger to allow
more call site IDs, if necessary.
o The callsite structures for a module will be freed when that module
is unloaded, even though sk_buffs may be using IDs corresponding to
those call sites. To allow useful error reporting, the call site
information in a module being unloaded is copied. If a module is
unloaded and CONFIG_CALLSITE_TERSE is enabled, the address of
the call site is not longer valid, so only the function name and
offset are printed. If CONFIG_CALLSITE_TERSE is enabled and the
is loaded, the address is also reported.

Signed-off-by: David VomLehn <[email protected]>


2010-06-11 20:38:02

by David Miller

[permalink] [raw]
Subject: Re: [PATCH 1/4][RFC] Introduction to sk_buff state checking


This is the second time you've submitted this patch set,
it is also the second time that:

1) Your email client has emitted these strange
"[email protected]"@cisco.com
addresses.

I saw that you were specifically notified about this
problem during the previous submission.

What did you do to try and correct it?

2) You are not CC:'ing the networking developer mailing
list "[email protected]" Many networking developers
do not read linux-kernel, so by not CC:'ing that list
you are not reaching the folks most capable of reviewing
your patches.

Please fix this up.

2010-06-11 20:46:47

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 1/4][RFC] Introduction to sk_buff state checking

On Fri, 11 Jun 2010 13:38:11 -0700 (PDT) David Miller wrote:

>
> This is the second time you've submitted this patch set,
> it is also the second time that:
>
> 1) Your email client has emitted these strange
> "[email protected]"@cisco.com
> addresses.
>
> I saw that you were specifically notified about this
> problem during the previous submission.
>
> What did you do to try and correct it?
>
> 2) You are not CC:'ing the networking developer mailing
> list "[email protected]" Many networking developers
> do not read linux-kernel, so by not CC:'ing that list
> you are not reaching the folks most capable of reviewing
> your patches.
>
> Please fix this up.
> --

Ack. Also this one:

----- The following addresses had permanent fatal errors -----
<[email protected]>
(reason: 550 Host unknown)

----- Transcript of session follows -----
550 5.1.2 <[email protected]>... Host unknown (Name server: dvomlehn-lnx2.corp.sa.net: host not found)


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2010-06-11 20:50:53

by David VomLehn

[permalink] [raw]
Subject: Re: [PATCH 1/4][RFC] Introduction to sk_buff state checking

On Fri, Jun 11, 2010 at 01:38:11PM -0700, David Miller wrote:
>
> This is the second time you've submitted this patch set,

There are changes, based on the feedback I got.

> it is also the second time that:
>
> 1) Your email client has emitted these strange
> "[email protected]"@cisco.com
> addresses.
>
> I saw that you were specifically notified about this
> problem during the previous submission.
>
> What did you do to try and correct it?
>
> 2) You are not CC:'ing the networking developer mailing
> list "[email protected]" Many networking developers
> do not read linux-kernel, so by not CC:'ing that list
> you are not reaching the folks most capable of reviewing
> your patches.

All true, and I apologize. I know there's enough traffic on kernel mailing
lists as it is.

> Please fix this up.

Working on it now.

--
David VL