On 16-03-01 09:00 AM, Amir Vadai wrote:
> On Tue, Mar 01, 2016 at 03:52:08PM +0100, Jiri Pirko wrote:
>> Tue, Mar 01, 2016 at 03:24:48PM CET, a...@vadai.me wrote:
>>> Extend ndo_setup_tc() to support ingress tc offloading. Will be used by
>>> later patches to offload tc flower filter.
>>>
>>> Feature is off by default and could be enabled by issuing:
>>> # ethtool -K eth0 hw-tc-offload on
>>>
>>> Offloads flow table is dynamically created when first filter is
>>> added.
>>> Rules are saved in a hash table that is maintained by the consumer (for
>>> example - the flower offload in the next patch).
>>> When last filter is removed and no filters exist in the hash table, the
>>> offload flow table is destroyed.
>>
>> <snip>
>>
>>> @@ -1880,6 +1883,17 @@ static int mlx5e_setup_tc(struct net_device *netdev,
>>> u8 tc)
>>> static int mlx5e_ndo_setup_tc(struct net_device *dev, u32 handle,
>>> __be16 proto, struct tc_to_netdev *tc)
>>> {
>>> + struct mlx5e_priv *priv = netdev_priv(dev);
>>> +
>>> + if (TC_H_MAJ(handle) != TC_H_MAJ(TC_H_INGRESS))
>>> + goto mqprio;
>>> +
>>> + switch (tc->type) {
>>> + default:
>>> + return -EINVAL;
>>
>> -EOPNOTSUPP would be better here perhaps?
>>
>>
>>> + }
>>> +
>>> +mqprio:
>>> if (handle != TC_H_ROOT || tc->type != TC_SETUP_MQPRIO)
>>> return -EINVAL;
>>>
>>> @@ -1963,6 +1977,13 @@ static int mlx5e_set_features(struct net_device
>>> *netdev,
>>> mlx5e_disable_vlan_filter(priv);
>>> }
>>>
>>> + if ((changes & NETIF_F_HW_TC) && !(features & NETIF_F_HW_TC) &&
>>> + mlx5e_tc_num_filters(priv)) {
>>> + netdev_err(netdev,
>>> + "Active offloaded tc filters, can't turn
>>> hw_tc_offload off\n");
>>> + return -EINVAL;
>>
>> This should not fail I believe. Just disable it in hw. I would even toss
>> away the rules if necessary.
> It depends on the answer regarding your comment on the previous patch.
> If we have the rule in both SW and HW, and remove it from the HW it is
> ok (although, currently I don't understand why would anyone want in both
> places).
> If the rule is processed by HW only - turning off this flag, will
> disable the offloaded rules - it might be misleading, so I prefered not
> to allow it and print a message.
When we get the HW only mode we will need to also flush the hardware
representation in software as well as the hardware state.
