Re: [RFC] ndo_validate_skb: Let the netdev check a valid skb content

[David Miller <davem@davemloft.net>]

From: Oliver Hartkopp <oliver@hartkopp.net>
Date: Wed, 23 Dec 2009 20:23:27 +0100

> @@ -2034,6 +2035,14 @@ int dev_queue_xmit(struct sk_buff *skb)
>  			goto out_kfree_skb;
>  	}
>  
> +	/* If the device offers a function to validate the skb content, let
> +	 * it check the skb and return an error to the caller if it fails.
> +	 */ 
> +	if (ops->ndo_validate_skb && ops->ndo_validate_skb(skb)) {
> +		rc = -EINVAL;
> +		goto out_kfree_skb;
> +	}
> +

To me this is as valuable as no error at all and simply having
the driver drop the frame.  Which is what we do now.  I can
sniff at the receiver to see that the device never transmitted
the frame.

Is this getting this generic -EINVAL back so important that it's
worth adding the new if() test for the NULL method to every packet
that traverses the machine?  Keep in mind we can route at rates
exceeding 1 million packets per second, so ever cycle you add
here really matters.

If you want to place the hooks still, put them out of the fast
path, which is probably in AF_PACKET.  We can extend this to
call the validation function from other "untrusted" sources.

But do realize that you're not saving anything, things like
queueing disciplines and traffic classifiers in the qdisc
layer can modify any part of the packet however they wish.

So even if the CAN protocol layer itself emitted a valid
frame, the qdisc layer can "corrupt" it.