Great, so needed changes are on their way, and i will issue tomorrow the A-MSDU patch that uses the new handlers scheme. thanks ron > > > > -int ieee80211_is_eapol(const struct sk_buff *skb); > > > > +int ieee80211_is_eapol(const struct sk_buff *skb, int hdrlen); > > > > > > That's just an optimisation, right? > > > > main reason is that ieee80211_is_eapol is called both in Tx and Rx > > flows, so the caller is reaponsible for passing the header length > > (802.3 in Rx and 802.11 in Tx). > > Right. I noticed later but forgot to remove the comment. > > > > Any reason you're getting rid of the MAC printk? It's a debug-only > > > message so whatever, just curious really. > > > > > > > as ieee80211_802_1x_pae is not a 802.11 specific any more, it has no knowledge > > about the header, therefore no knowledge about mac address. I could > > pass to it the mac address as well, but looks odd to do that only for > > debug printing. > > Oh ok, right. > > > > > + if (ieee80211_data_to_8023(rx) == TXRX_DROP) > > > > + return TXRX_DROP; > > > > + if (ieee80211_802_1x_pae(rx, sizeof(struct ethhdr)) == TXRX_DROP) > > > > + return TXRX_DROP; > > > > + if (ieee80211_drop_unencrypted(rx, sizeof(struct ethhdr)) == TXRX_DROP) > > > > + return TXRX_DROP; > > > > > > The sizeof() doesn't look right, shouldn't that get the hdrlen from the > > > frame control? Ah, no, I see now, it's already converted at this point. > > > Do we need to pass the hdrlen at all then? I think it would make sense > > > to specify (in comments) that those functions get a converted (to 802.3 > > > framing) skb instead. And then hardcode the frame offsets/header length. > > > > those functions can be used now for 802.11 or 802.3 frames so caller > > should pass header length for them > > > > > > > > Also, I think the return value of those functions shouldn't be the > > > txrx_result but rather just a bool. > > > > > > > Hmmm... yes, it will probebly look better. i'll just do a slight > > change in function's names so it will be clearer to reader. > > Great, thanks. > > > > I think it would make sense to have another function > > > ieee80211_is_eapol_checked(tx->skb) that does the hdrlen calculation to > > > save doing it unconditionally here for the (common) case where we're > > > using a sta or default key. > > > > I agree, thanks for the remark. > > Yet, i wouldn't like to add another function that does basically the > > same as ieee80211_is_eapol (or sending an indication for > > ieee80211_is_eapol about header type what will lessen its > > flexsibility) > > I was thinking to move ieee80211_get_hdrlen to: > > ieee80211_is_eapol(tx->skb, ieee80211_get_hdrlen(fc)) > > this way i keep flexibility in ieee80211_is_eapol and won't calculate > > header length unless really needed. > > Oh sure, that works too. I guess you could inline all of it even > ieee80211_is_eapol(tx->skb, > ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control))) > > But yeah, whatever, I'm not concerned about the byteswap. > > johannes > > - To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
