* Re: [PATCH 1/3] [LLC]: skb allocation size for responses [not found] <1206691159-10872-1-git-send-email-joonwpark81@gmail.com> @ 2008-03-28 13:12 ` Arnaldo Carvalho de Melo 2008-03-29 6:06 ` Joonwoo Park 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-03-28 13:12 UTC (permalink / raw) To: Joonwoo Park; +Cc: davem, netdev Em Fri, Mar 28, 2008 at 04:59:19PM +0900, Joonwoo Park escreveu: > allocate the skb for llc responses with the received packet size by > using the size adjustable llc_frame_alloc. > don't allocate useless extra payload. > cleanup magic numbers > > Signed-off-by: Joonwoo Park <joonwpark81@gmail.com> <SNIP> > diff --git a/net/llc/llc_sap.c b/net/llc/llc_sap.c > index 2525165..46447e7 100644 > --- a/net/llc/llc_sap.c > +++ b/net/llc/llc_sap.c > @@ -24,20 +24,52 @@ > #include <net/tcp_states.h> > #include <linux/llc.h> > > +#ifdef CONFIG_TR > +static inline int __llc_tr_mac_header_len(unsigned short devtype) > +{ > + if (devtype == ARPHDR_IEEE802_TR) > + return sizeof(struct thr_hdr); > + return 0; > +} > +#else > +static inline int __llc_tr_mac_header_len(unsigned short devtype) > +{ > + return 0; > +} > +#endif Why not just: static inline int __llc_tr_mac_header_len(unsigned short devtype) { #ifdef CONFIG_TR if (devtype == ARPHDR_IEEE802_TR) return sizeof(struct thr_hdr); #endif return 0; } ? > +static int llc_mac_header_len(unsigned short devtype) > +{ > + switch (devtype) { > + case ARPHRD_ETHER: > + case ARPHRD_LOOPBACK: > + return sizeof(struct ethhdr); > + } > + > + return __llc_tr_mac_header_len(devtype); > +} Nah, just drop __llc_tr_mac_header_len altogether and have another case entry for ARPHDR_IEEE802_TR. > /** > * llc_alloc_frame - allocates sk_buff for frame > * @dev: network device this skb will be sent over > + * @type: pdu type to allocate > + * @data_size: data 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, > + u8 type, u32 data_size) > { > - struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC); > + int hlen = type == LLC_PDU_TYPE_U ? 3 : 4; > + struct sk_buff *skb; > + > + hlen += llc_mac_header_len(dev->type); > + skb = alloc_skb(hlen + data_size, GFP_ATOMIC); > > if (skb) { > skb_reset_mac_header(skb); > - skb_reserve(skb, 50); > + skb_reserve(skb, hlen); Well done, now it is better documented (LLC_PDU_TYPE_U passed) and uses less memory, no more magic numbers, good. - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-03-28 13:12 ` [PATCH 1/3] [LLC]: skb allocation size for responses Arnaldo Carvalho de Melo @ 2008-03-29 6:06 ` Joonwoo Park 2008-03-29 12:02 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 8+ messages in thread From: Joonwoo Park @ 2008-03-29 6:06 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: davem, netdev, Joonwoo Park On Fri, Mar 28, 2008 at 10:12:02AM -0300, Arnaldo Carvalho de Melo wrote: > > > +static int llc_mac_header_len(unsigned short devtype) > > +{ > > + switch (devtype) { > > + case ARPHRD_ETHER: > > + case ARPHRD_LOOPBACK: > > + return sizeof(struct ethhdr); > > + } > > + > > + return __llc_tr_mac_header_len(devtype); > > +} > > Nah, just drop __llc_tr_mac_header_len altogether and have another case > entry for ARPHDR_IEEE802_TR. > > > /** > > * llc_alloc_frame - allocates sk_buff for frame > > * @dev: network device this skb will be sent over > > + * @type: pdu type to allocate > > + * @data_size: data 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, > > + u8 type, u32 data_size) > > { > > - struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC); > > + int hlen = type == LLC_PDU_TYPE_U ? 3 : 4; > > + struct sk_buff *skb; > > + > > + hlen += llc_mac_header_len(dev->type); > > + skb = alloc_skb(hlen + data_size, GFP_ATOMIC); > > > > if (skb) { > > skb_reset_mac_header(skb); > > - skb_reserve(skb, 50); > > + skb_reserve(skb, hlen); > > Well done, now it is better documented (LLC_PDU_TYPE_U passed) and uses > less memory, no more magic numbers, good. > > - Arnaldo I've respined the patch, thanks Arnaldo. --- [LLC]: skb allocation size for responses Allocate the skb for llc responses with the received packet size by using the size adjustable llc_frame_alloc. Don't allocate useless extra payload. Cleanup magic numbers. So, this fixes oops. 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 <joonwpark81@gmail.com> --- include/net/llc_pdu.h | 4 ++-- include/net/llc_sap.h | 7 +++++-- net/llc/llc_c_ac.c | 47 +++++++++++++++++++++++++---------------------- net/llc/llc_pdu.c | 2 +- net/llc/llc_s_ac.c | 9 +++++++-- net/llc/llc_sap.c | 27 ++++++++++++++++++++++++--- net/llc/llc_station.c | 13 ++++++++++--- 7 files changed, 74 insertions(+), 35 deletions(-) diff --git a/include/net/llc_pdu.h b/include/net/llc_pdu.h index 4a8f58b..75b8e29 100644 --- a/include/net/llc_pdu.h +++ b/include/net/llc_pdu.h @@ -381,7 +381,7 @@ static inline void llc_pdu_init_as_xid_cmd(struct sk_buff *skb, xid_info->fmt_id = LLC_XID_FMT_ID; /* 0x81 */ xid_info->type = svcs_supported; xid_info->rw = rx_window << 1; /* size of receive window */ - skb_put(skb, 3); + skb_put(skb, sizeof(struct llc_xid_info)); } /** @@ -406,7 +406,7 @@ static inline void llc_pdu_init_as_xid_rsp(struct sk_buff *skb, xid_info->fmt_id = LLC_XID_FMT_ID; xid_info->type = svcs_supported; xid_info->rw = rx_window << 1; - skb_put(skb, 3); + skb_put(skb, sizeof(struct llc_xid_info)); } /* LLC Type 2 FRMR response information field format */ diff --git a/include/net/llc_sap.h b/include/net/llc_sap.h index 2c56dbe..ed25bec 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> @@ -19,8 +22,8 @@ struct sock; 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); +extern struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev, + u8 type, u32 data_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..71a0022 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, LLC_PDU_TYPE_U, 0); 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, LLC_PDU_TYPE_U, 0); 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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -282,7 +282,8 @@ 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, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; @@ -306,7 +307,8 @@ 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, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; @@ -336,7 +338,8 @@ int llc_conn_ac_resend_frmr_rsp_f_set_p(struct sock *sk, struct sk_buff *skb) struct llc_sock *llc = llc_sk(sk); llc_pdu_decode_pf_bit(skb, &f_bit); - nskb = llc_alloc_frame(sk, llc->dev); + nskb = llc_alloc_frame(sk, llc->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb); @@ -424,7 +427,7 @@ int llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr(struct sock *sk, struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -459,7 +462,7 @@ int llc_conn_ac_send_rej_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -483,7 +486,7 @@ int llc_conn_ac_send_rej_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -507,7 +510,7 @@ int llc_conn_ac_send_rej_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -531,7 +534,7 @@ int llc_conn_ac_send_rnr_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -555,7 +558,7 @@ int llc_conn_ac_send_rnr_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -579,7 +582,7 @@ int llc_conn_ac_send_rnr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -615,7 +618,7 @@ int llc_conn_ac_opt_send_rnr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -639,7 +642,7 @@ int llc_conn_ac_send_rr_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -663,7 +666,7 @@ int llc_conn_ac_send_rr_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -688,7 +691,7 @@ int llc_conn_ac_send_ack_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -712,7 +715,7 @@ int llc_conn_ac_send_rr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -736,7 +739,7 @@ int llc_conn_ac_send_ack_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -770,7 +773,7 @@ int llc_conn_ac_send_sabme_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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -799,7 +802,7 @@ int llc_conn_ac_send_ua_rsp_f_set_p(struct sock *sk, struct sk_buff *skb) u8 f_bit; 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, LLC_PDU_TYPE_U, 0); llc_pdu_decode_pf_bit(skb, &f_bit); if (nskb) { @@ -956,7 +959,7 @@ static int llc_conn_ac_send_rr_rsp_f_set_ackpf(struct sock *sk, { 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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; diff --git a/net/llc/llc_pdu.c b/net/llc/llc_pdu.c index fa83243..2e6cb79 100644 --- a/net/llc/llc_pdu.c +++ b/net/llc/llc_pdu.c @@ -241,7 +241,7 @@ void llc_pdu_init_as_frmr_rsp(struct sk_buff *skb, struct llc_pdu_sn *prev_pdu, FRMR_INFO_SET_PDU_INFO_2LONG_IND(frmr_info, vzyxw); FRMR_INFO_SET_PDU_INVALID_Nr_IND(frmr_info, vzyxw); FRMR_INFO_SET_PDU_INVALID_Ns_IND(frmr_info, vzyxw); - skb_put(skb, 5); + skb_put(skb, sizeof(struct llc_frmr_info)); } /** diff --git a/net/llc/llc_s_ac.c b/net/llc/llc_s_ac.c index ac3d93b..a94bd56 100644 --- a/net/llc/llc_s_ac.c +++ b/net/llc/llc_s_ac.c @@ -103,7 +103,8 @@ int llc_sap_action_send_xid_r(struct llc_sap *sap, struct sk_buff *skb) 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); + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, sap->laddr.lsap, dsap, @@ -144,11 +145,15 @@ 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 data_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); + + /* The test request command is type U (llc_len = 3) */ + data_size = ntohs(eth_hdr(skb)->h_proto) - 3; + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, data_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..65799b1 100644 --- a/net/llc/llc_sap.c +++ b/net/llc/llc_sap.c @@ -24,20 +24,41 @@ #include <net/tcp_states.h> #include <linux/llc.h> +static int llc_mac_header_len(unsigned short devtype) +{ + switch (devtype) { + case ARPHRD_ETHER: + case ARPHRD_LOOPBACK: + return sizeof(struct ethhdr); +#ifdef CONFIG_TR + case ARPHDR_IEEE802_TR: + return sizeof(struct thr_hdr); +#endif + } + return 0; +} + /** * llc_alloc_frame - allocates sk_buff for frame * @dev: network device this skb will be sent over + * @type: pdu type to allocate + * @data_size: data 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, + u8 type, u32 data_size) { - struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC); + int hlen = type == LLC_PDU_TYPE_U ? 3 : 4; + struct sk_buff *skb; + + hlen += llc_mac_header_len(dev->type); + skb = alloc_skb(hlen + data_size, GFP_ATOMIC); if (skb) { skb_reset_mac_header(skb); - skb_reserve(skb, 50); + skb_reserve(skb, hlen); skb_reset_network_header(skb); skb_reset_transport_header(skb); skb->protocol = htons(ETH_P_802_2); diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c index 959e7f3..83da133 100644 --- a/net/llc/llc_station.c +++ b/net/llc/llc_station.c @@ -253,7 +253,8 @@ static int llc_station_ac_inc_xid_r_cnt_by_1(struct sk_buff *skb) static int llc_station_ac_send_null_dsap_xid_c(struct sk_buff *skb) { int rc = 1; - struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev); + struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; @@ -274,7 +275,8 @@ static int llc_station_ac_send_xid_r(struct sk_buff *skb) { u8 mac_da[ETH_ALEN], dsap; int rc = 1; - struct sk_buff* nskb = llc_alloc_frame(NULL, skb->dev); + struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; @@ -298,7 +300,12 @@ static int llc_station_ac_send_test_r(struct sk_buff *skb) { u8 mac_da[ETH_ALEN], dsap; int rc = 1; - struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev); + u32 data_size; + struct sk_buff *nskb; + + /* The test request command is type U (llc_len = 3) */ + data_size = ntohs(eth_hdr(skb)->h_proto) - 3; + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, data_size); if (!nskb) goto out; -- 1.5.4.3 --- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-03-29 6:06 ` Joonwoo Park @ 2008-03-29 12:02 ` Arnaldo Carvalho de Melo 2008-04-01 2:27 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-03-29 12:02 UTC (permalink / raw) To: Joonwoo Park; +Cc: davem, netdev Em Sat, Mar 29, 2008 at 03:06:55PM +0900, Joonwoo Park escreveu: > On Fri, Mar 28, 2008 at 10:12:02AM -0300, Arnaldo Carvalho de Melo wrote: > > > > > +static int llc_mac_header_len(unsigned short devtype) > > > +{ > > > + switch (devtype) { > > > + case ARPHRD_ETHER: > > > + case ARPHRD_LOOPBACK: > > > + return sizeof(struct ethhdr); > > > + } > > > + > > > + return __llc_tr_mac_header_len(devtype); > > > +} > > > > Nah, just drop __llc_tr_mac_header_len altogether and have another case > > entry for ARPHDR_IEEE802_TR. > > > > > /** > > > * llc_alloc_frame - allocates sk_buff for frame > > > * @dev: network device this skb will be sent over > > > + * @type: pdu type to allocate > > > + * @data_size: data 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, > > > + u8 type, u32 data_size) > > > { > > > - struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC); > > > + int hlen = type == LLC_PDU_TYPE_U ? 3 : 4; > > > + struct sk_buff *skb; > > > + > > > + hlen += llc_mac_header_len(dev->type); > > > + skb = alloc_skb(hlen + data_size, GFP_ATOMIC); > > > > > > if (skb) { > > > skb_reset_mac_header(skb); > > > - skb_reserve(skb, 50); > > > + skb_reserve(skb, hlen); > > > > Well done, now it is better documented (LLC_PDU_TYPE_U passed) and uses > > less memory, no more magic numbers, good. > > > > - Arnaldo > > > I've respined the patch, thanks Arnaldo. > > --- > [LLC]: skb allocation size for responses > > Allocate the skb for llc responses with the received packet size by > using the size adjustable llc_frame_alloc. > Don't allocate useless extra payload. > Cleanup magic numbers. > > So, this fixes oops. > 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! Perfect, thanks a lot: Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-03-29 12:02 ` Arnaldo Carvalho de Melo @ 2008-04-01 2:27 ` David Miller 2008-04-01 3:45 ` Joonwoo Park 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2008-04-01 2:27 UTC (permalink / raw) To: acme; +Cc: joonwpark81, netdev From: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Sat, 29 Mar 2008 09:02:23 -0300 > Perfect, thanks a lot: > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> Breaks the build: net/llc/llc_sap.c: In function 'llc_mac_header_len': net/llc/llc_sap.c:34: error: 'ARPHDR_IEEE802_TR' undeclared (first use in this function) net/llc/llc_sap.c:34: error: (Each undeclared identifier is reported only once net/llc/llc_sap.c:34: error: for each function it appears in.) net/llc/llc_sap.c:35: error: invalid application of 'sizeof' to incomplete type 'struct thr_hdr' ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-04-01 2:27 ` David Miller @ 2008-04-01 3:45 ` Joonwoo Park 2008-04-01 4:06 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Joonwoo Park @ 2008-04-01 3:45 UTC (permalink / raw) To: David Miller; +Cc: acme, joonwpark81, netdev On Mon, Mar 31, 2008 at 07:27:30PM -0700, David Miller wrote: > From: Arnaldo Carvalho de Melo <acme@redhat.com> > Date: Sat, 29 Mar 2008 09:02:23 -0300 > > > Perfect, thanks a lot: > > > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> > > Breaks the build: Ahhhhhhh. Sorry guys. > > net/llc/llc_sap.c: In function 'llc_mac_header_len': > net/llc/llc_sap.c:34: error: 'ARPHDR_IEEE802_TR' undeclared (first use in this function) > net/llc/llc_sap.c:34: error: (Each undeclared identifier is reported only once > net/llc/llc_sap.c:34: error: for each function it appears in.) > net/llc/llc_sap.c:35: error: invalid application of 'sizeof' to incomplete type 'struct thr_hdr' should be : ARPHDR -> ARPHRD, thr -> trh --- [LLC]: skb allocation size for responses Allocate the skb for llc responses with the received packet size by using the size adjustable llc_frame_alloc. Don't allocate useless extra payload. Cleanup magic numbers. So, this fixes oops. 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 <joonwpark81@gmail.com> Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- include/net/llc_pdu.h | 4 ++-- include/net/llc_sap.h | 7 +++++-- net/llc/llc_c_ac.c | 47 +++++++++++++++++++++++++---------------------- net/llc/llc_pdu.c | 2 +- net/llc/llc_s_ac.c | 9 +++++++-- net/llc/llc_sap.c | 27 ++++++++++++++++++++++++--- net/llc/llc_station.c | 13 ++++++++++--- 7 files changed, 74 insertions(+), 35 deletions(-) diff --git a/include/net/llc_pdu.h b/include/net/llc_pdu.h index 4a8f58b..75b8e29 100644 --- a/include/net/llc_pdu.h +++ b/include/net/llc_pdu.h @@ -381,7 +381,7 @@ static inline void llc_pdu_init_as_xid_cmd(struct sk_buff *skb, xid_info->fmt_id = LLC_XID_FMT_ID; /* 0x81 */ xid_info->type = svcs_supported; xid_info->rw = rx_window << 1; /* size of receive window */ - skb_put(skb, 3); + skb_put(skb, sizeof(struct llc_xid_info)); } /** @@ -406,7 +406,7 @@ static inline void llc_pdu_init_as_xid_rsp(struct sk_buff *skb, xid_info->fmt_id = LLC_XID_FMT_ID; xid_info->type = svcs_supported; xid_info->rw = rx_window << 1; - skb_put(skb, 3); + skb_put(skb, sizeof(struct llc_xid_info)); } /* LLC Type 2 FRMR response information field format */ diff --git a/include/net/llc_sap.h b/include/net/llc_sap.h index 2c56dbe..ed25bec 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> @@ -19,8 +22,8 @@ struct sock; 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); +extern struct sk_buff *llc_alloc_frame(struct sock *sk, struct net_device *dev, + u8 type, u32 data_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..71a0022 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, LLC_PDU_TYPE_U, 0); 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, LLC_PDU_TYPE_U, 0); 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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -282,7 +282,8 @@ 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, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; @@ -306,7 +307,8 @@ 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, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; @@ -336,7 +338,8 @@ int llc_conn_ac_resend_frmr_rsp_f_set_p(struct sock *sk, struct sk_buff *skb) struct llc_sock *llc = llc_sk(sk); llc_pdu_decode_pf_bit(skb, &f_bit); - nskb = llc_alloc_frame(sk, llc->dev); + nskb = llc_alloc_frame(sk, llc->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_frmr_info)); if (nskb) { struct llc_sap *sap = llc->sap; struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(skb); @@ -424,7 +427,7 @@ int llc_conn_ac_resend_i_xxx_x_set_0_or_send_rr(struct sock *sk, struct llc_pdu_sn *pdu = llc_pdu_sn_hdr(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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -459,7 +462,7 @@ int llc_conn_ac_send_rej_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -483,7 +486,7 @@ int llc_conn_ac_send_rej_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -507,7 +510,7 @@ int llc_conn_ac_send_rej_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -531,7 +534,7 @@ int llc_conn_ac_send_rnr_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -555,7 +558,7 @@ int llc_conn_ac_send_rnr_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -579,7 +582,7 @@ int llc_conn_ac_send_rnr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -615,7 +618,7 @@ int llc_conn_ac_opt_send_rnr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -639,7 +642,7 @@ int llc_conn_ac_send_rr_cmd_p_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -663,7 +666,7 @@ int llc_conn_ac_send_rr_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -688,7 +691,7 @@ int llc_conn_ac_send_ack_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -712,7 +715,7 @@ int llc_conn_ac_send_rr_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -736,7 +739,7 @@ int llc_conn_ac_send_ack_xxx_x_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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -770,7 +773,7 @@ int llc_conn_ac_send_sabme_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, LLC_PDU_TYPE_U, 0); if (nskb) { struct llc_sap *sap = llc->sap; @@ -799,7 +802,7 @@ int llc_conn_ac_send_ua_rsp_f_set_p(struct sock *sk, struct sk_buff *skb) u8 f_bit; 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, LLC_PDU_TYPE_U, 0); llc_pdu_decode_pf_bit(skb, &f_bit); if (nskb) { @@ -956,7 +959,7 @@ static int llc_conn_ac_send_rr_rsp_f_set_ackpf(struct sock *sk, { 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, LLC_PDU_TYPE_S, 0); if (nskb) { struct llc_sap *sap = llc->sap; diff --git a/net/llc/llc_pdu.c b/net/llc/llc_pdu.c index fa83243..2e6cb79 100644 --- a/net/llc/llc_pdu.c +++ b/net/llc/llc_pdu.c @@ -241,7 +241,7 @@ void llc_pdu_init_as_frmr_rsp(struct sk_buff *skb, struct llc_pdu_sn *prev_pdu, FRMR_INFO_SET_PDU_INFO_2LONG_IND(frmr_info, vzyxw); FRMR_INFO_SET_PDU_INVALID_Nr_IND(frmr_info, vzyxw); FRMR_INFO_SET_PDU_INVALID_Ns_IND(frmr_info, vzyxw); - skb_put(skb, 5); + skb_put(skb, sizeof(struct llc_frmr_info)); } /** diff --git a/net/llc/llc_s_ac.c b/net/llc/llc_s_ac.c index ac3d93b..a94bd56 100644 --- a/net/llc/llc_s_ac.c +++ b/net/llc/llc_s_ac.c @@ -103,7 +103,8 @@ int llc_sap_action_send_xid_r(struct llc_sap *sap, struct sk_buff *skb) 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); + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; llc_pdu_header_init(nskb, LLC_PDU_TYPE_U, sap->laddr.lsap, dsap, @@ -144,11 +145,15 @@ 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 data_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); + + /* The test request command is type U (llc_len = 3) */ + data_size = ntohs(eth_hdr(skb)->h_proto) - 3; + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, data_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..65799b1 100644 --- a/net/llc/llc_sap.c +++ b/net/llc/llc_sap.c @@ -24,20 +24,41 @@ #include <net/tcp_states.h> #include <linux/llc.h> +static int llc_mac_header_len(unsigned short devtype) +{ + switch (devtype) { + case ARPHRD_ETHER: + case ARPHRD_LOOPBACK: + return sizeof(struct ethhdr); +#ifdef CONFIG_TR + case ARPHRD_IEEE802_TR: + return sizeof(struct trh_hdr); +#endif + } + return 0; +} + /** * llc_alloc_frame - allocates sk_buff for frame * @dev: network device this skb will be sent over + * @type: pdu type to allocate + * @data_size: data 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, + u8 type, u32 data_size) { - struct sk_buff *skb = alloc_skb(128, GFP_ATOMIC); + int hlen = type == LLC_PDU_TYPE_U ? 3 : 4; + struct sk_buff *skb; + + hlen += llc_mac_header_len(dev->type); + skb = alloc_skb(hlen + data_size, GFP_ATOMIC); if (skb) { skb_reset_mac_header(skb); - skb_reserve(skb, 50); + skb_reserve(skb, hlen); skb_reset_network_header(skb); skb_reset_transport_header(skb); skb->protocol = htons(ETH_P_802_2); diff --git a/net/llc/llc_station.c b/net/llc/llc_station.c index 959e7f3..83da133 100644 --- a/net/llc/llc_station.c +++ b/net/llc/llc_station.c @@ -253,7 +253,8 @@ static int llc_station_ac_inc_xid_r_cnt_by_1(struct sk_buff *skb) static int llc_station_ac_send_null_dsap_xid_c(struct sk_buff *skb) { int rc = 1; - struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev); + struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; @@ -274,7 +275,8 @@ static int llc_station_ac_send_xid_r(struct sk_buff *skb) { u8 mac_da[ETH_ALEN], dsap; int rc = 1; - struct sk_buff* nskb = llc_alloc_frame(NULL, skb->dev); + struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, + sizeof(struct llc_xid_info)); if (!nskb) goto out; @@ -298,7 +300,12 @@ static int llc_station_ac_send_test_r(struct sk_buff *skb) { u8 mac_da[ETH_ALEN], dsap; int rc = 1; - struct sk_buff *nskb = llc_alloc_frame(NULL, skb->dev); + u32 data_size; + struct sk_buff *nskb; + + /* The test request command is type U (llc_len = 3) */ + data_size = ntohs(eth_hdr(skb)->h_proto) - 3; + nskb = llc_alloc_frame(NULL, skb->dev, LLC_PDU_TYPE_U, data_size); if (!nskb) goto out; -- 1.5.4.3 --- ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-04-01 3:45 ` Joonwoo Park @ 2008-04-01 4:06 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2008-04-01 4:06 UTC (permalink / raw) To: joonwpark81; +Cc: acme, netdev From: Joonwoo Park <joonwpark81@gmail.com> Date: Tue, 1 Apr 2008 12:45:09 +0900 > should be : ARPHDR -> ARPHRD, thr -> trh > > --- > [LLC]: skb allocation size for responses Applied, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <47e76779.0f98600a.36ef.ffff9c13@mx.google.com>]
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses [not found] <47e76779.0f98600a.36ef.ffff9c13@mx.google.com> @ 2008-03-24 17:16 ` Arnaldo Carvalho de Melo 2008-03-25 5:15 ` Joonwoo Park 0 siblings, 1 reply; 8+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-03-24 17:16 UTC (permalink / raw) To: joonwpark81; +Cc: davem, netdev, jwestfall Em Mon, Mar 24, 2008 at 05:33:57PM +0900, joonwpark81@gmail.com escreveu: > From: Joonwoo Park <joonwpark81@gmail.com> > > 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 <joonwpark81@gmail.com> > --- > 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) > 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 > 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 > 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 > 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> > @@ -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. > + 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. <SNIP> - Arnaldo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] [LLC]: skb allocation size for responses 2008-03-24 17:16 ` Arnaldo Carvalho de Melo @ 2008-03-25 5:15 ` Joonwoo Park 0 siblings, 0 replies; 8+ messages in thread From: Joonwoo Park @ 2008-03-25 5:15 UTC (permalink / raw) To: Arnaldo Carvalho de Melo; +Cc: joonwpark81, davem, netdev, jwestfall 2008/3/25, Arnaldo Carvalho de Melo <acme@redhat.com>: > Em Mon, Mar 24, 2008 at 05:33:57PM +0900, joonwpark81@gmail.com escreveu: > > From: Joonwoo Park <joonwpark81@gmail.com> > > > > 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 <joonwpark81@gmail.com> > > --- > > 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. > > > 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 > > > 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 > > > 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 > > > 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> > > > @@ -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 > > > > + 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-04-01 4:06 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1206691159-10872-1-git-send-email-joonwpark81@gmail.com>
2008-03-28 13:12 ` [PATCH 1/3] [LLC]: skb allocation size for responses Arnaldo Carvalho de Melo
2008-03-29 6:06 ` Joonwoo Park
2008-03-29 12:02 ` Arnaldo Carvalho de Melo
2008-04-01 2:27 ` David Miller
2008-04-01 3:45 ` Joonwoo Park
2008-04-01 4:06 ` David Miller
[not found] <47e76779.0f98600a.36ef.ffff9c13@mx.google.com>
2008-03-24 17:16 ` Arnaldo Carvalho de Melo
2008-03-25 5:15 ` Joonwoo Park
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.