[Intel-wired-lan] [PATCH next-queue v1 2/5] igc: Integrate flex filter into ethtool ops

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Hi Tony,

"Nguyen, Anthony L" <anthony.l.nguyen@intel.com> writes:

> On Thu, 2021-06-10 at 17:39 -0700, Vinicius Costa Gomes wrote:
>> From: Kurt Kanzenbach <kurt@linutronix.de>
>>
>> Use the flex filter mechanism to extend the current ethtool filter
>> operations by intercoperating the user data. This allows to match
>> eight more bytes within a Ethernet frame in addition to macs, ether
>> types and vlan.
>>
>> The matching pattern looks like this:
>>
>>  * dest_mac [6]
>>  * src_mac [6]
>>  * tpid [2]
>>  * vlan tci [2]
>>  * ether type [2]
>>  * user data [8]
>>
>> This can be used to match Profinet traffic classes by FrameID range.
>>
>> Signed-off-by: Kurt Kanzenbach <kurt@linutronix.de>
>> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>> ---
>
> I was preparing these patches for a pull request and came across a
> couple warnings. Could you address them before I send these on?

Sure thing. Sorry I missed those.

>
> <snip>
>
>> @@ -1215,6 +1221,20 @@ static void igc_ethtool_init_nfc_rule(struct
>> igc_nfc_rule *rule,
>>               ether_addr_copy(rule->filter.dst_addr,
>>                               fsp->h_u.ether_spec.h_dest);
>>       }
>> +
>> +     /* Check for user defined data */
>> +     if ((fsp->flow_type & FLOW_EXT) &&
>> +         (fsp->h_ext.data[0] || fsp->h_ext.data[1])) {
>> +             rule->filter.match_flags |= IGC_FILTER_FLAG_USER_DATA;
>> +             memcpy(rule->filter.user_data, fsp->h_ext.data,
>> sizeof(fsp->h_ext.data));
>> +             memcpy(rule->filter.user_mask, fsp->m_ext.data,
>> sizeof(fsp->m_ext.data));
>> +
>> +             /* VLAN etype matching is only valid using flex filter
>> */
>> +             if ((fsp->flow_type & FLOW_EXT) && fsp-
>> >h_ext.vlan_etype) {
>> +                     rule->filter.vlan_etype = fsp-
>> >h_ext.vlan_etype;
>> +                     rule->filter.match_flags |=
>> IGC_FILTER_FLAG_VLAN_ETYPE;
>
> drivers/net/ethernet/intel/igc/igc_ethtool.c:1234:49: warning:
> incorrect type in assignment (different base types)
> drivers/net/ethernet/intel/igc/igc_ethtool.c:1234:49:    expected
> unsigned short [usertype] vlan_etype
> drivers/net/ethernet/intel/igc/igc_ethtool.c:1234:49:    got restricted
> __be16 const [usertype] vlan_etype

Will fix.

>
> <snip>
>
>> +static int igc_add_flex_filter(struct igc_adapter *adapter,
>> +                            struct igc_nfc_rule *rule)
>> +{
>> +     struct igc_flex_filter flex = { };
>> +     struct igc_nfc_filter *filter = &rule->filter;
>> +     unsigned int eth_offset, user_offset;
>> +     int ret, index;
>> +     bool vlan;
>> +
>> +     index = igc_find_avail_flex_filter_slot(adapter);
>> +     if (index < 0)
>> +             return -ENOSPC;
>> +
>> +     /* Construct the flex filter:
>> +      *  -> dest_mac [6]
>> +      *  -> src_mac [6]
>> +      *  -> tpid [2]
>> +      *  -> vlan tci [2]
>> +      *  -> ether type [2]
>> +      *  -> user data [8]
>> +      *  -> = 26 bytes => 32 length
>> +      */
>> +     flex.index    = index;
>> +     flex.length   = 32;
>> +     flex.rx_queue = rule->action;
>> +
>> +     vlan = rule->filter.vlan_tci || rule->filter.vlan_etype;
>> +     eth_offset = vlan ? 16 : 12;
>> +     user_offset = vlan ? 18 : 14;
>> +
>> +     /* Add destination MAC  */
>> +     if (rule->filter.match_flags & IGC_FILTER_FLAG_DST_MAC_ADDR)
>> +             igc_flex_filter_add_field(&flex, &filter->dst_addr, 0,
>> +                                       ETH_ALEN, NULL);
>> +
>> +     /* Add source MAC */
>> +     if (rule->filter.match_flags & IGC_FILTER_FLAG_SRC_MAC_ADDR)
>> +             igc_flex_filter_add_field(&flex, &filter->src_addr, 6,
>> +                                       ETH_ALEN, NULL);
>> +
>> +     /* Add VLAN etype */
>> +     if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_ETYPE)
>> +             igc_flex_filter_add_field(&flex, &filter->vlan_etype,
>> 12,
>> +                                       sizeof(filter->vlan_etype),
>> +                                       NULL);
>> +
>> +     /* Add VLAN TCI */
>> +     if (rule->filter.match_flags & IGC_FILTER_FLAG_VLAN_TCI)
>> +             igc_flex_filter_add_field(&flex, &filter->vlan_tci, 14,
>> +                                       sizeof(filter->vlan_tci),
>> NULL);
>> +
>> +     /* Add Ether type */
>> +     if (rule->filter.match_flags & IGC_FILTER_FLAG_ETHER_TYPE) {
>> +             u16 etype = cpu_to_be16(filter->etype);
>
> drivers/net/ethernet/intel/igc/igc_main.c:3332:29: warning: incorrect
> type in initializer (different base types)
> drivers/net/ethernet/intel/igc/igc_main.c:3332:29:    expected unsigned
> short [usertype] etype
> drivers/net/ethernet/intel/igc/igc_main.c:3332:29:    got restricted
> __be16 [usertype]

Will fix.

>
>
>> +             igc_flex_filter_add_field(&flex, &etype, eth_offset,
>> +                                       sizeof(etype), NULL);
>> +     }
>>


Cheers,
-- 
Vinicius