Thread (2 messages) 2 messages, 2 authors, 2008-03-25

Re: [PATCH 1/3] [LLC]: skb allocation size for responses

From: Joonwoo Park <hidden>
Date: 2008-03-25 05:15:35

Possibly related (same subject, not in this thread)

2008/3/25, Arnaldo Carvalho de Melo [off-list ref]:
Em Mon, Mar 24, 2008 at 05:33:57PM +0900, joonwpark81@gmail.com escreveu:
quoted
From: Joonwoo Park <redacted>

allocate the skb for llc responses with the received packet size by
using the size adjustable llc_frame_alloc.

Reported by Jim Westfall:
kernel: skb_over_panic: text:c0541fc7 len:1000 put:997 head:c166ac00 data:c166ac2f tail:0xc166b017 end:0xc166ac80 dev:eth0
kernel: ------------[ cut here ]------------
kernel: kernel BUG at net/core/skbuff.c:95!

Signed-off-by: Joonwoo Park <redacted>
---
diff --git a/include/net/llc_sap.h b/include/net/llc_sap.h
index 2c56dbe..a5c9f5b 100644
--- a/include/net/llc_sap.h
+++ b/include/net/llc_sap.h
@@ -1,5 +1,8 @@
 #ifndef LLC_SAP_H
 #define LLC_SAP_H
+
+#include <asm/types.h>
+
 /*
  * Copyright (c) 1997 by Procom Technology,Inc.
  *            2001-2003 by Arnaldo Carvalho de Melo <acme@conectiva.com.br>
@@ -20,7 +23,7 @@ extern void llc_sap_rtn_pdu(struct llc_sap *sap, struct sk_buff *skb);
 extern void llc_save_primitive(struct sock *sk, struct sk_buff* skb,
                             unsigned char prim);
 extern struct sk_buff *llc_alloc_frame(struct sock *sk,
-                                    struct net_device *dev);
+                                    struct net_device *dev, u32 size);

 extern void llc_build_and_send_test_pkt(struct llc_sap *sap,
                                      struct sk_buff *skb,
diff --git a/net/llc/llc_c_ac.c b/net/llc/llc_c_ac.c
index 860140c..a9db49d 100644
--- a/net/llc/llc_c_ac.c
+++ b/net/llc/llc_c_ac.c
@@ -198,7 +198,7 @@ int llc_conn_ac_send_disc_cmd_p_set_x(struct sock *sk, struct sk_buff *skb)
 {
      int rc = -ENOBUFS;
      struct llc_sock *llc = llc_sk(sk);
-     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev);
+     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, 78);
I know that this ancient code has magic numbers, but I guess we
shouldn't introduce new ones :-\ Why 78? (rethoric)
In fact, I'd like to ask it to you. Why 128? (rhetoric #2) :-)
Anyway, It's my bad, I was a thoughtless calculator who can only
subtraction (128 - 50 = 78)
I'll elminiate old & new magic number(s) at next try.
quoted
      if (nskb) {
              struct llc_sap *sap = llc->sap;
@@ -223,7 +223,7 @@ int llc_conn_ac_send_dm_rsp_f_set_p(struct sock *sk, struct sk_buff *skb)
 {
      int rc = -ENOBUFS;
      struct llc_sock *llc = llc_sk(sk);
-     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev);
+     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, 78);
ditto
quoted
      if (nskb) {
              struct llc_sap *sap = llc->sap;
@@ -249,7 +249,7 @@ int llc_conn_ac_send_dm_rsp_f_set_1(struct sock *sk, struct sk_buff *skb)
 {
      int rc = -ENOBUFS;
      struct llc_sock *llc = llc_sk(sk);
-     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev);
+     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, 78);
ditto
quoted
      if (nskb) {
              struct llc_sap *sap = llc->sap;
@@ -282,7 +282,7 @@ int llc_conn_ac_send_frmr_rsp_f_set_x(struct sock *sk, struct sk_buff *skb)
              llc_pdu_decode_pf_bit(skb, &f_bit);
      else
              f_bit = 0;
-     nskb = llc_alloc_frame(sk, llc->dev);
+     nskb = llc_alloc_frame(sk, llc->dev, 78);
ditto
quoted
      if (nskb) {
              struct llc_sap *sap = llc->sap;
@@ -306,7 +306,7 @@ int llc_conn_ac_resend_frmr_rsp_f_set_0(struct sock *sk, struct sk_buff *skb)
 {
      int rc = -ENOBUFS;
      struct llc_sock *llc = llc_sk(sk);
-     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev);
+     struct sk_buff *nskb = llc_alloc_frame(sk, llc->dev, 78);
ditto


<SNIP>
quoted
@@ -144,11 +144,18 @@ int llc_sap_action_send_test_r(struct llc_sap *sap, struct sk_buff *skb)
      u8 mac_da[ETH_ALEN], mac_sa[ETH_ALEN], dsap;
      struct sk_buff *nskb;
      int rc = 1;
+     u32 size;

      llc_pdu_decode_sa(skb, mac_da);
      llc_pdu_decode_da(skb, mac_sa);
      llc_pdu_decode_ssap(skb, &dsap);
-     nskb = llc_alloc_frame(NULL, skb->dev);
+
+#ifdef NET_SKBUFF_DATA_USES_OFFSET
+     size = skb->end + skb->data_len;
+#else
+     size = skb->end - skb->head;
+#endif

huh? Try not to use NET_SKBUFF_DATA_USES_OFFSET, it should die at some
point, perhaps today :-)

Please use one of the existing helpers.
I see. Thanks
quoted
+     nskb = llc_alloc_frame(NULL, skb->dev, size);
      if (!nskb)
              goto out;
      llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, sap->laddr.lsap, dsap,
diff --git a/net/llc/llc_sap.c b/net/llc/llc_sap.c
index 2525165..e295549 100644
--- a/net/llc/llc_sap.c
+++ b/net/llc/llc_sap.c
@@ -27,13 +27,15 @@
 /**
  *   llc_alloc_frame - allocates sk_buff for frame
  *   @dev: network device this skb will be sent over
+ *   @size: size to allocate
  *
  *   Allocates an sk_buff for frame and initializes sk_buff fields.
  *   Returns allocated skb or %NULL when out of memory.
  */
-struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev)
+struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev,
+                             u32 size)
 {
-     struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC);
+     struct sk_buff *skb = alloc_skb(size + 50, GFP_ATOMIC);
take the opportunity to document what is fifty in this context.
I see. Thanks,
<SNIP>

- Arnaldo
Thanks,
Joonwoo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help