Re: [patch net-next v4 6/7] rocker: remove nowait from switchdev callbacks.

[Scott Feldman]

On Mon, Oct 12, 2015 at 11:25 PM, Jiri Pirko  wrote:
> Tue, Oct 13, 2015 at 06:02:28AM CEST, sfel...@gmail.com wrote:
>>On Mon, Oct 12, 2015 at 11:03 AM, Jiri Pirko  wrote:
>>> From: Jiri Pirko 
>>>
>>> No need to avoid sleeping in switchdev callbacks now, as the switchdev
>>> core allows it.
>>>
>>> Signed-off-by: Jiri Pirko 
>>> ---
>>>  drivers/net/ethernet/rocker/rocker.c | 7 +++
>>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/net/ethernet/rocker/rocker.c 
>>> b/drivers/net/ethernet/rocker/rocker.c
>>> index bb956a5..9629c5b5 100644
>>> --- a/drivers/net/ethernet/rocker/rocker.c
>>> +++ b/drivers/net/ethernet/rocker/rocker.c
>>> @@ -3672,7 +3672,7 @@ static int rocker_port_fdb_flush(struct rocker_port 
>>> *rocker_port,
>>> rocker_port->stp_state == BR_STATE_FORWARDING)
>>> return 0;
>>>
>>> -   flags |= ROCKER_OP_FLAG_REMOVE;
>>> +   flags |= ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;
>>
>>I understand the two changes below where you're removing NOWAIT, but
>>here you're adding NOWAIT which I'm not sure how that is related to
>>the switchdev core changes.  Is this two patches?
>
> I removed ROCKER_OP_FLAG_NOWAIT from attr_set. But here in
> rocker_port_fdb_flush, which is called from attr_set, we call
> rocker_port_fdb_learn with spin lock. Therefore I had to put
> ROCKER_OP_FLAG_NOWAIT here. Before ROCKER_OP_FLAG_NOWAIT removal from
> attr_set this was there already.

Gotcha.

Acked-by: Scott Feldman 
--
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

Re: [patch net-next v4 6/7] rocker: remove nowait from switchdev callbacks.

[Jiri Pirko]

Tue, Oct 13, 2015 at 06:02:28AM CEST, sfel...@gmail.com wrote:
>On Mon, Oct 12, 2015 at 11:03 AM, Jiri Pirko  wrote:
>> From: Jiri Pirko 
>>
>> No need to avoid sleeping in switchdev callbacks now, as the switchdev
>> core allows it.
>>
>> Signed-off-by: Jiri Pirko 
>> ---
>>  drivers/net/ethernet/rocker/rocker.c | 7 +++
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/rocker/rocker.c 
>> b/drivers/net/ethernet/rocker/rocker.c
>> index bb956a5..9629c5b5 100644
>> --- a/drivers/net/ethernet/rocker/rocker.c
>> +++ b/drivers/net/ethernet/rocker/rocker.c
>> @@ -3672,7 +3672,7 @@ static int rocker_port_fdb_flush(struct rocker_port 
>> *rocker_port,
>> rocker_port->stp_state == BR_STATE_FORWARDING)
>> return 0;
>>
>> -   flags |= ROCKER_OP_FLAG_REMOVE;
>> +   flags |= ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;
>
>I understand the two changes below where you're removing NOWAIT, but
>here you're adding NOWAIT which I'm not sure how that is related to
>the switchdev core changes.  Is this two patches?

I removed ROCKER_OP_FLAG_NOWAIT from attr_set. But here in
rocker_port_fdb_flush, which is called from attr_set, we call
rocker_port_fdb_learn with spin lock. Therefore I had to put
ROCKER_OP_FLAG_NOWAIT here. Before ROCKER_OP_FLAG_NOWAIT removal from
attr_set this was there already.


>
>
>> spin_lock_irqsave(>fdb_tbl_lock, lock_flags);
>>
>> @@ -4382,8 +4382,7 @@ static int rocker_port_attr_set(struct net_device *dev,
>>
>> switch (attr->id) {
>> case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
>> -   err = rocker_port_stp_update(rocker_port, trans,
>> -ROCKER_OP_FLAG_NOWAIT,
>> +   err = rocker_port_stp_update(rocker_port, trans, 0,
>>  attr->u.stp_state);
>> break;
>> case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
>> @@ -4517,7 +4516,7 @@ static int rocker_port_fdb_del(struct rocker_port 
>> *rocker_port,
>>const struct switchdev_obj_port_fdb *fdb)
>>  {
>> __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, 
>> NULL);
>> -   int flags = ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;
>> +   int flags = ROCKER_OP_FLAG_REMOVE;
>>
>> if (!rocker_port_is_bridged(rocker_port))
>> return -EINVAL;
>> --
>> 1.9.3
>>
--
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

Re: [patch net-next v4 6/7] rocker: remove nowait from switchdev callbacks.

[Scott Feldman]

On Mon, Oct 12, 2015 at 11:03 AM, Jiri Pirko  wrote:
> From: Jiri Pirko 
>
> No need to avoid sleeping in switchdev callbacks now, as the switchdev
> core allows it.
>
> Signed-off-by: Jiri Pirko 
> ---
>  drivers/net/ethernet/rocker/rocker.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/rocker/rocker.c 
> b/drivers/net/ethernet/rocker/rocker.c
> index bb956a5..9629c5b5 100644
> --- a/drivers/net/ethernet/rocker/rocker.c
> +++ b/drivers/net/ethernet/rocker/rocker.c
> @@ -3672,7 +3672,7 @@ static int rocker_port_fdb_flush(struct rocker_port 
> *rocker_port,
> rocker_port->stp_state == BR_STATE_FORWARDING)
> return 0;
>
> -   flags |= ROCKER_OP_FLAG_REMOVE;
> +   flags |= ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;

I understand the two changes below where you're removing NOWAIT, but
here you're adding NOWAIT which I'm not sure how that is related to
the switchdev core changes.  Is this two patches?


> spin_lock_irqsave(>fdb_tbl_lock, lock_flags);
>
> @@ -4382,8 +4382,7 @@ static int rocker_port_attr_set(struct net_device *dev,
>
> switch (attr->id) {
> case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
> -   err = rocker_port_stp_update(rocker_port, trans,
> -ROCKER_OP_FLAG_NOWAIT,
> +   err = rocker_port_stp_update(rocker_port, trans, 0,
>  attr->u.stp_state);
> break;
> case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> @@ -4517,7 +4516,7 @@ static int rocker_port_fdb_del(struct rocker_port 
> *rocker_port,
>const struct switchdev_obj_port_fdb *fdb)
>  {
> __be16 vlan_id = rocker_port_vid_to_vlan(rocker_port, fdb->vid, NULL);
> -   int flags = ROCKER_OP_FLAG_NOWAIT | ROCKER_OP_FLAG_REMOVE;
> +   int flags = ROCKER_OP_FLAG_REMOVE;
>
> if (!rocker_port_is_bridged(rocker_port))
> return -EINVAL;
> --
> 1.9.3
>
--
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