| Message ID | 1487896109-23851-7-git-send-email-greearb@candelatech.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
And here's the third patch in a row modifying the same code ... johannes
On 24 February 2017 at 01:28, <greearb@candelatech.com> wrote: > Modify the receive-from-user-space logic to do length > and 'is-down' checks before trying to allocate an skb. > > And, if we are going to ignore the pkt due to radio idle, > then do not return an error code to user-space. User-space > cannot reliably know exactly when a radio is idle or not. You probably want to return some error code anyway because 0, if you compare to the kernel medium, currently maps to the ack returned bit and is possibly the only way for userspace to set the HWSIM_TX_STAT_ACK flag in a meaningful way. Best regards
On 02/24/2017 12:45 AM, Andrew Zaborowski wrote: > On 24 February 2017 at 01:28, <greearb@candelatech.com> wrote: >> Modify the receive-from-user-space logic to do length >> and 'is-down' checks before trying to allocate an skb. >> >> And, if we are going to ignore the pkt due to radio idle, >> then do not return an error code to user-space. User-space >> cannot reliably know exactly when a radio is idle or not. > > You probably want to return some error code anyway because 0, if you > compare to the kernel medium, currently maps to the ack returned bit > and is possibly the only way for userspace to set the > HWSIM_TX_STAT_ACK flag in a meaningful way. Maybe there is a way to return a specific error code so that the user-space doesn't get concerned when radio is idle. I didn't want to spam logs in user-space app... Thanks, Ben
diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index 48ddf5d..3a96933 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -3020,25 +3020,6 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2, frame_data_len = nla_len(info->attrs[HWSIM_ATTR_FRAME]); frame_data = (void *)nla_data(info->attrs[HWSIM_ATTR_FRAME]); - /* Allocate new skb here */ - skb = alloc_skb(frame_data_len, GFP_KERNEL); - if (skb == NULL) { - if (net_ratelimit()) - printk(KERN_DEBUG " hwsim rx-nl: skb alloc failed, len: %d\n", - frame_data_len); - goto out; - } - - if (frame_data_len > IEEE80211_MAX_DATA_LEN) { - if (net_ratelimit()) - printk(KERN_DEBUG " hwsim rx-nl: data lenth error: %d max: %d\n", - frame_data_len, IEEE80211_MAX_DATA_LEN); - goto out; - } - - /* Copy the data */ - memcpy(skb_put(skb, frame_data_len), frame_data, frame_data_len); - data2 = get_hwsim_data_ref_from_addr(dst); if (!data2) { @@ -3067,9 +3048,33 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2, if (((cnt++ & 0x3FF) == 0x3FF) && net_ratelimit()) printk(KERN_DEBUG " hwsim rx-nl: radio %pM idle: %d or not started: %d cnt: %d\n", dst, data2->idle, !data2->started, cnt); + /* Fail silently, no need to bug user-space about this, since lots of races + * in up/down interface, and the user-space app cannot keep perfectly + * in sync. + */ + return 0; + } + + if (frame_data_len > IEEE80211_MAX_DATA_LEN) { + if (net_ratelimit()) + printk(KERN_DEBUG " hwsim rx-nl: data lenth error: %d max: %d\n", + frame_data_len, IEEE80211_MAX_DATA_LEN); + goto out; + } + + + /* Allocate new skb here */ + skb = alloc_skb(frame_data_len, GFP_KERNEL); + if (skb == NULL) { + if (net_ratelimit()) + printk(KERN_DEBUG " hwsim rx-nl: skb alloc failed, len: %d\n", + frame_data_len); goto out; } + /* Copy the data */ + memcpy(skb_put(skb, frame_data_len), frame_data, frame_data_len); + /* A frame is received from user space */ memset(&rx_status, 0, sizeof(rx_status)); if (info->attrs[HWSIM_ATTR_FREQ]) {