On Thu, Jun 16, 2016 at 1:18 PM, Tom Herbert <t...@herbertland.com> wrote: > On Thu, Jun 16, 2016 at 11:58 AM, Alexander Duyck > <alexander.du...@gmail.com> wrote: >> On Thu, Jun 16, 2016 at 10:51 AM, Tom Herbert <t...@herbertland.com> wrote: >>> This replaces gso_type SKB_GSO_DODGY with a new tx_flag named >>> SKBTX_UNTRUSTED_SOURCE. This more generically desrcibes the skb >>> being created from a untrusted source as a characteristic of and skbuff. >>> This also frees up one gso_type flag bit. >>> >>> Signed-off-by: Tom Herbert <t...@herbertland.com> >> >> Instead of leaving this bit in the shared_info why not look at moving >> it into the sk_buff itself? It seems like this might be a better >> candidate for something like that as a large part of what the dodgy >> bit represents is that the header offsets are likely not set correctly >> and need to be parsed out and updated. It might make more sense to >> place this in the slot just after remcsum_offload. That way once all >> the header offsets have been updated you could just update this one >> bit to indicate that the header offsets stored in this sk_buff are >> valid. >> >> I also don't see where these changes address any changes needed to >> skb_gso_ok in order to actually trigger the partial walk though the >> GSO code. You probably need to look at adding a statement there to do >> a check for your untrusted source bit versus the GSO_ROBUST feature. >> It probably doesn't need to be much, just something like tacking on a >> "&& (!skb_is_untrustued(skb) || (features & NETIF_F_GSO_ROBUST)" to >> the conditional statement. >> > All the places where SKB_GSO_DODGY was being checked should have been > replaced with SKBTX_UNTRUSTED_SOURCE.
Yes and no. So net_gso_ok used to be one of those places but you moved the one bit and dropped the other so is it no longer is being checked there. That is why I mentioned looking at adding one additional check for the untrusted bit and NETIF_F_GSO_ROBUST. You could probably just tack it on at the end of the features check at the end of net_gso_ok. - Alex
