Re: [PATCHv2 net] i40e: Implement ndo_gso_check()

[Joe Stringer]

On 20 November 2014 16:19, Jesse Gross <jesse@xxxxxxxxxx> wrote:
> On Thu, Nov 20, 2014 at 3:11 PM, Joe Stringer <joestringer@xxxxxxxxxx> wrote:
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index c3a7f4a..2b01c8d 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -7447,6 +7447,36 @@ static int i40e_ndo_fdb_dump(struct sk_buff *skb,
>>
>>  #endif /* USE_DEFAULT_FDB_DEL_DUMP */
>>  #endif /* HAVE_FDB_OPS */
>> +static bool i40e_gso_check(struct sk_buff *skb, struct net_device *dev)
>> +{
>> +       if ((skb_shinfo(skb)->gso_type & SKB_GSO_IPIP) &&
>> +           (skb->inner_protocol_type != ENCAP_TYPE_IPPROTO ||
>> +            skb->inner_protocol != htons(IPPROTO_IPIP))) {
>
> I think this check on inner_protocol isn't really semantically correct
> - that field is a union with inner_ipproto, so it would be more
> correct to check against that and then you wouldn't need to use htons
> either.

Yes, translation error on my part, but if IPIP isn't exposed I'll just
drop this section.

> I don't know if we need to have the check at all for IPIP though -
> after all the driver doesn't expose support for it all (actually it
> doesn't expose GRE either). This raises kind of an interesting
> question about the checks though - it's pretty easy to add support to
> the driver for a new GSO type (and I imagine that people will be
> adding GRE soon) and forget to update the check.

If the check is more conservative, then testing would show that it's
not working and lead people to figure out why (and update the check).

>> +       if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE | SKB_GSO_UDP_TUNNEL)) {
>> +               unsigned char *ihdr;
>> +
>> +               if (skb->inner_protocol_type != ENCAP_TYPE_ETHER)
>> +                       return false;
>
> I guess if you want to be nice, it might be good to check the
> equivalent IP protocol types as well.

Can do (just UDP as I'll drop GRE as per the above).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/