Re: [PATCH 1/4] backports: replace igb skb->no_fcs patch with semantic patch

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03.06.2015 22:13, Hauke Mehrtens wrote:
> On 06/03/2015 01:25 PM, Stefan Assmann wrote:
>> Signed-off-by: Stefan Assmann <sassmann@xxxxxxxxx>
>> ---
>>  .../network/0035-skb_no_fcs/igb_skb_no_fcs.patch           | 14 --------------
>>  .../network/0035-skb_no_fcs/skb_no_fcs.cocci               |  7 +++++++
>>  2 files changed, 7 insertions(+), 14 deletions(-)
>>  delete mode 100644 patches/collateral-evolutions/network/0035-skb_no_fcs/igb_skb_no_fcs.patch
>>  create mode 100644 patches/collateral-evolutions/network/0035-skb_no_fcs/skb_no_fcs.cocci
>>
>> diff --git a/patches/collateral-evolutions/network/0035-skb_no_fcs/igb_skb_no_fcs.patch b/patches/collateral-evolutions/network/0035-skb_no_fcs/igb_skb_no_fcs.patch
>> deleted file mode 100644
>> index f659bfd..0000000
>> --- a/patches/collateral-evolutions/network/0035-skb_no_fcs/igb_skb_no_fcs.patch
>> +++ /dev/null
>> @@ -1,14 +0,0 @@
>> ---- a/drivers/net/ethernet/intel/igb/igb_main.c
>> -+++ b/drivers/net/ethernet/intel/igb/igb_main.c
>> -@@ -4782,9 +4782,10 @@ static u32 igb_tx_cmd_type(struct sk_buf
>> - 	cmd_type |= IGB_SET_FLAG(tx_flags, IGB_TX_FLAGS_TSTAMP,
>> - 				 (E1000_ADVTXD_MAC_TSTAMP));
>> - 
>> -+#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,4,0)
>> - 	/* insert frame checksum */
>> - 	cmd_type ^= IGB_SET_FLAG(skb->no_fcs, 1, E1000_ADVTXD_DCMD_IFCS);
>> --
>> -+#endif /* LINUX_VERSION_CODE >= KERNEL_VERSION(3,4,0) */
>> - 	return cmd_type;
>> - }
>> - 
>> diff --git a/patches/collateral-evolutions/network/0035-skb_no_fcs/skb_no_fcs.cocci b/patches/collateral-evolutions/network/0035-skb_no_fcs/skb_no_fcs.cocci
>> new file mode 100644
>> index 0000000..703c227
>> --- /dev/null
>> +++ b/patches/collateral-evolutions/network/0035-skb_no_fcs/skb_no_fcs.cocci
>> @@ -0,0 +1,7 @@
>> +@r1@
>> +expression E1,E2;
>> +struct sk_buff *skb;
>> +@@
>> ++#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,18,0)
>> + E1 ^= E2(..., skb->no_fcs, ...)
>> ++#endif /* if LINUX_VERSION_CODE >= KERNEL_VERSION(3,18,0) */
>>
> 
> Is it always save to just remove something which accesses skb->no_fcs in
> all cases? I think sometimes some special handling for older kernel
> version could be needed. This also looks very specific to the igb usage.
> 
> Hauke
> 

In this case I'd say this is fine, no_fcs is used to send out frames with
bad CRC for testing. So just commenting out related code shouldn't cause
any harm. Yes, the cocci code is very specific and will likely need to be
extended for other drivers when we pull them in. But you have to start
somewhere.

We always have the option to revert if something turns out to be a bad
idea.

  Stefan
--
To unsubscribe from this list: send the line "unsubscribe backports" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux