>-----Original Message----- >From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] >Sent: Friday, March 3, 2017 6:09 PM >To: Pubbisetty, Manikanta <mpubbise@xxxxxxxxxxxxxxxx> >Cc: linux-wireless@xxxxxxxxxxxxxxx; Thiagarajan, Vasanthakumar ><vthiagar@xxxxxxxxxxxxxxxx> >Subject: Re: [RFCv2 2/2] mac80211: Implement data xmit for 802.11 encap >offload > > >> There is a field, no_80211_encap, added to ieee80211_tx_info:control >> to mark if the 802.11 encapsulation is offloaded to driver. > >Why is that needed? Since you have a separate TX path (ndo_start_xmit), >wouldn't it make more sense to call into a drv_tx_8023() or something like >that instead? > [Manikanta] Yes, it makes sense having a separate driver hook instead of using drv_tx, but in case of drivers (fe ath10k) where enqueuing and dequeuing tx logic (wake_tx_queue and ieee80211_tx_dequeue) is used there should be a way to indicate that the packet is not 80211 encapsulated, isn't it? Correct me if I am missing anything. >> There is also a new callback for tx completion status indication to >> handle data frames using 802.11 encap offload. > >Maybe you could just use _noskb? > [Manikanta] Hmmm you are right. I just went through the code, seems ieee80211_tx_status_noskb does most of them what ieee80211_tx_status_8023 is doing, resetting the station connection monitoring logic and updating last known tx rate is missing in _noskb. These are required, isn't it? >Haven't really looked at the rest all that much, few comments: > > * not sure you're handling non-linear frames right, are you assuming > the driver can handle them? probably a fair assumption, but should > be documented > [Manikanta] Sure > * you seem to also be assuming that the driver not only does all > encryption in HW (which is obviously needed) but also does all the > key lookups etc. - also seems fair, but also should be documented > [Manikanta] I will document these in my next version. > * similarly for a lot of other (all?) fields in tx_info > * you seem to be assuming that if encap offload is supported then it's > also *desired* for AP/VLAN and client interfaces, if not 4-addr. > This seems ... probably about right, but if drivers don't always > support it? Or support it in more cases? Perhaps we can move the > SUPPORTS_80211_ENCAP flag into a VIF flag instead, so they can do it > more fine-grained? > [Manikanta] You are correct. It is good if driver advertises the vif types it support in encap offload mode, but how can we decide the netdev_ops to be used while creating the vif? drv_add_interface will be invoked from ndo_open and even before invoking driver add_interface we have to make the decision of using Ethernet mode netdev_ops or the default netdev_ops, isn't it? Can we have a separate driver hook which gives whether 80211 encap offload is supported for the given interface type and then decide the netdev_ops to be attached for the vif? -- Manikanta
