From: Neil H. <nh...@tu...> - 2005-09-27 18:03:50
|
On Mon, Sep 26, 2005 at 08:38:29PM -0400, Neil Horman wrote: > On Mon, Sep 26, 2005 at 03:02:59PM -0700, Sridhar Samudrala wrote: ><snip> > >=20 > > I agree that it is much cleaner if we could use the destructor. If you = can > > do some testing with a capture program running in background and valida= te > > that there are no issues, i am OK with using skb->destructor. > >=20 > I'll roll that patch and test with sctp_darn and ethereal in the morning. >=20 Hey- I rerolled this patch to use do receive accounting in sctp_ulpevent_set_owner, and to undo the receive accounting in the skb's destructor. I'm able to monitor a connection via tcpdump while running sctp_darn and the sctp regression test suite with no failures, and correct accounting. While I was making these changes I also removed the sctp recei= ve buffer accounting from sctp_rcv in input.c. This lets me consolidate all r= eceive accounting in sctp_ulpevent_set_owner without incurring the inadvertent dou= ble counting of received frames that get cloned in sctp_ulpevent_make_rcvmsg. All tested by me, using sctp_darn and the lksctp-tools test suite in conjun= ction with tcpdump, with good results. Thanks and regards Neil Signed-off-by: Neil Horman <nh...@tu...> input.c | 17 ----------------- ulpevent.c | 36 +++++++++++++++++++++--------------- 2 files changed, 21 insertions(+), 32 deletions(-) --- linux-2.6/net/sctp/input.c.orig 2005-09-27 08:31:51.000000000 -0400 +++ linux-2.6/net/sctp/input.c 2005-09-27 10:56:47.000000000 -0400 @@ -100,21 +100,6 @@ static inline int sctp_rcv_checksum(stru return 0; } =20 -/* The free routine for skbuffs that sctp receives */ -static void sctp_rfree(struct sk_buff *skb) -{ - atomic_sub(sizeof(struct sctp_chunk),&skb->sk->sk_rmem_alloc); - sock_rfree(skb); -} - -/* The ownership wrapper routine to do receive buffer accounting */ -static void sctp_rcv_set_owner_r(struct sk_buff *skb, struct sock *sk) -{ - skb_set_owner_r(skb,sk); - skb->destructor =3D sctp_rfree; - atomic_add(sizeof(struct sctp_chunk),&sk->sk_rmem_alloc); -} - struct sctp_input_cb { union { struct inet_skb_parm h4; @@ -256,8 +241,6 @@ int sctp_rcv(struct sk_buff *skb) } SCTP_INPUT_CB(skb)->chunk =3D chunk; =20 - sctp_rcv_set_owner_r(skb,sk); - /* Remember what endpoint is to handle this packet. */ chunk->rcvr =3D rcvr; =20 --- linux-2.6/net/sctp/ulpevent.c.orig 2005-09-27 08:30:27.000000000 -0400 +++ linux-2.6/net/sctp/ulpevent.c 2005-09-27 11:14:57.000000000 -0400 @@ -52,19 +52,6 @@ static void sctp_ulpevent_receive_data(s struct sctp_association *asoc); static void sctp_ulpevent_release_data(struct sctp_ulpevent *event); =20 -/* Stub skb destructor. */ -static void sctp_stub_rfree(struct sk_buff *skb) -{ -/* WARNING: This function is just a warning not to use the - * skb destructor. If the skb is shared, we may get the destructor - * callback on some processor that does not own the sock_lock. This - * was occuring with PACKET socket applications that were monitoring - * our skbs. We can't take the sock_lock, because we can't risk - * recursing if we do really own the sock lock. Instead, do all - * of our rwnd manipulation while we own the sock_lock outright. - */ -} - /* Initialize an ULP event from an given skb. */ SCTP_STATIC void sctp_ulpevent_init(struct sctp_ulpevent *event, int msg_f= lags) { @@ -98,6 +85,26 @@ int sctp_ulpevent_is_notification(const=20 return MSG_NOTIFICATION =3D=3D (event->msg_flags & MSG_NOTIFICATION); } =20 +/* The free routine for skbuffs that sctp receives */ +static void sctp_rfree(struct sk_buff *skb) +{ + struct sctp_ulpevent *event =3D sctp_skb2event(skb); + if (!(event->msg_flags & MSG_NOTIFICATION)) + atomic_sub(sizeof(struct sctp_chunk),&skb->sk->sk_rmem_alloc); + sock_rfree(skb); +} + +/* The ownership wrapper routine to do receive buffer accounting */ +static void sctp_rcv_set_owner_r(struct sk_buff *skb, struct sock *sk) +{ + struct sctp_ulpevent *event =3D sctp_skb2event(skb); + + skb_set_owner_r(skb,sk); + skb->destructor =3D sctp_rfree; + if (!(event->msg_flags & MSG_NOTIFICATION)) + atomic_add(sizeof(struct sctp_chunk),&sk->sk_rmem_alloc); +} + /* Hold the association in case the msg_name needs read out of * the association. */ @@ -111,9 +118,8 @@ static inline void sctp_ulpevent_set_own */ sctp_association_hold((struct sctp_association *)asoc); skb =3D sctp_event2skb(event); - skb->sk =3D asoc->base.sk; event->asoc =3D (struct sctp_association *)asoc; - skb->destructor =3D sctp_stub_rfree; + sctp_rcv_set_owner_r(skb,asoc->base.sk); } =20 /* A simple destructor to give up the reference to the association. */ --=20 /*************************************************** *Neil Horman *Software Engineer *gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu ***************************************************/ |