Thu, Oct 08, 2015 at 08:03:35AM CEST, sfel...@gmail.com wrote:
>On Wed, Oct 7, 2015 at 10:39 PM, Jiri Pirko <j...@resnulli.us> wrote:
>> Thu, Oct 08, 2015 at 06:27:07AM CEST, sfel...@gmail.com wrote:
>>>On Wed, Oct 7, 2015 at 11:30 AM, Jiri Pirko <j...@resnulli.us> wrote:
>>>> From: Jiri Pirko <j...@mellanox.com>
>>>>
>>>> Caller should know if he can call attr_set directly (when holding RTNL)
>>>> or if he has to use deferred version of this function.
>>>>
>>>> This also allows drivers to sleep inside attr_set and report operation
>>>> status back to switchdev core. Switchdev core then warns if status is
>>>> not ok, instead of silent errors happening in drivers.
>>>>
>>>> Signed-off-by: Jiri Pirko <j...@mellanox.com>
>>>> ---
>>>> include/net/switchdev.h | 2 +
>>>> net/bridge/br_stp.c | 4 +-
>>>> net/switchdev/switchdev.c | 113
>>>> +++++++++++++++++++++++++---------------------
>>>> 3 files changed, 65 insertions(+), 54 deletions(-)
>>>>
>>>> diff --git a/include/net/switchdev.h b/include/net/switchdev.h
>>>> index 89266a3..320be44 100644
>>>> --- a/include/net/switchdev.h
>>>> +++ b/include/net/switchdev.h
>>>> @@ -168,6 +168,8 @@ int switchdev_port_attr_get(struct net_device *dev,
>>>> struct switchdev_attr *attr);
>>>> int switchdev_port_attr_set(struct net_device *dev,
>>>> struct switchdev_attr *attr);
>>>> +int switchdev_port_attr_set_deferred(struct net_device *dev,
>>>> + struct switchdev_attr *attr);
>>>
>>>Rather than adding another op, use attr->flags and define:
>>>
>>>#define SWITCHDEV_F_DEFERRED BIT(x)
>>>
>>>So we get:
>>>
>>>void br_set_state(struct net_bridge_port *p, unsigned int state)
>>>{
>>> struct switchdev_attr attr = {
>>> .id = SWITCHDEV_ATTR_ID_PORT_STP_STATE,
>>>+ .flags = SWITCHDEV_F_DEFERRED,
>>> .u.stp_state = state,
>>> };
>>> int err;
>>>
>>> p->state = state;
>>> err = switchdev_port_attr_set(p->dev, &attr);
>>> if (err && err != -EOPNOTSUPP)
>>> br_warn(p->br, "error setting offload STP state on
>>>port %u(%s)\n",
>>> (unsigned int) p->port_no,
>>>p->dev->name);
>>>}
>>>
>>>(And add obj->flags to do the same).
>>
>> That's what I wanted to avoid. Also because the obj is const and for
>> call from work, this flag would have to be removed.
>
>What did you want to avoid?
Having this as a flag. I don't like it too much.
But that is cosmetics. Other than that, does the patchset make sense?
Do you see some possible issues?
Thanks.
Jiri
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
