On Wed, 18 Nov 1998, klein fabien wrote:

> here is the transmit function of the linux dlci.c module:
> 
> 
> static int dlci_transmit(struct sk_buff *skb, struct device *dev)
> {
>    struct dlci_local *dlp;
>    int               ret;
> 
>    ret = 0;
> 
>    if (!skb || !dev)
>       return(0);
> 
>    if (dev->tbusy)
>       return(1);

This defensiveness is understandable, but why would you be called if your tbusy
is 1? I'd almost be tempted to put in a panic() if that happens, since it's a
protocol violation. It could indicate that you are being called by a subsystem
that has no business calling you.

>    dlp = dev->priv;
> 
>    if (set_bit(0, (void*)&dev->tbusy) != 0)

You don't need this, unless you call this function from an interrupt
service routine. If it's just called at bottom-half time, there is no
need for atomic operations. You can just say:

                dev->tbusy = 1;

You know that tbusy was previously zero, so there is no need for
the if ().

>       /* Alan Cox recommends always returning 0, and always freeing
> the packet */
>       /* experience suggest a slightly more conservative approach */

You return 0 to indicate that you have accepted the packet. You don't have
to process it right away. You can do your own queueing. If you reject
the packet by returning 1, you don't free it. The layer above will
re-queue it and try to give it to you later.

But the only reason for wanting do do your own queuing is if you need to do
some special processing of the data for which you need access to a large enough
window of packets.

> But when this list is full, I return DLCI_RET_DROP.... and then the
> kernel crashes...damned!!!
> 
> Is the dev->tbusy =1  missing in the dlci module??

Apparently yes. If you return 1 from hard_xmit and don't set tbusy to 1, you
will create an non-preemptable infinite loop in your bottom-half processing
(read: system lockup requiring hard reboot). 

This tbusy stuff is awkward. From a clean interface design prespective, I would
say that it should not be in the interface at all.  All it is is an
optimization. If the device layer code sees that your tbusy flag is 1, it does
not try to shove you data, thereby avoiding some queue operations and a
function call. 

The tbusy flag should not be exposed to the device driver writers; it should be
automatically set to 1 when hard_xmit returns 1 to indicate that it's rejecting
the packet (thus avoiding the lock up error). And there should be a function
that resets it, perhaps called dev_transmit_ready() that the driver calls when
it's ready to accept more data. Failing to call dev_transmit_ready() will
simply cause network traffic to that device to stop, which is a recoverable
condition, unlike a system lockup.

Then again, the tbusy stuff is sort of a rite of passage for network driver
hackers, so I say keep it in! :) :) :)

-
To unsubscribe from this list: send the line "unsubscribe linux-net" in
the body of a message to [EMAIL PROTECTED]

Reply via email to