Re: [ovs-dev] [PATCH V7 1/7] ofproto: Consider datapath_type when looking for internal ports.

[Thadeu Lima de Souza Cascardo]

On Wed, Aug 10, 2016 at 10:09:07AM +, Kavanagh, Mark B wrote:
> >
> >On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo"  
> >wrote:
> >
> >>On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote:
> >>> From: Daniele Di Proietto 
> >>>
> >>> Interfaces with type "internal" end up having a netdev with type "tap"
> >>> in the dpif-netdev datapath, so a strcmp will fail to match internal
> >>> interfaces.
> >>>
> >>> We can translate the types with ofproto_port_open_type() before calling
> >>> strcmp to fix this.
> >>>
> >>> This fixes a minor issue where internal interfaces are considered
> >>> non-internal in the userspace datapath for the purpose of adjusting the
> >>> MTU.
> >>>
> >>> Signed-off-by: Daniele Di Proietto 
> >>
> >>Acked-by: Thadeu Lima de Souza Cascardo 
> >
> >Thanks guys, I pushed this to master
> >
> >>
> >>Hi, Mark.
> >>
> >>Can you keep my Ack in further submissions in case there are no changes to 
> >>this
> >>patch?
> 
> 
> Hi Cascardo,
> 
> Apologies - I performed the rework for the entire series on my local branch, 
> so your Ack wasn't present.
> 
> Thanks to Daniele for re-adding it.
> 
> Cheers,
> Mark

Hi, Mark.

No apologies needed. Just thought it could be missed and adding it to further
submissions could help with that.

Thanks.
Cascardo.

> 
> >>
> >>Thanks.
> >>Cascardo.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V7 1/7] ofproto: Consider datapath_type when looking for internal ports.

[Kavanagh, Mark B]

>
>On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo"  
>wrote:
>
>>On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote:
>>> From: Daniele Di Proietto 
>>>
>>> Interfaces with type "internal" end up having a netdev with type "tap"
>>> in the dpif-netdev datapath, so a strcmp will fail to match internal
>>> interfaces.
>>>
>>> We can translate the types with ofproto_port_open_type() before calling
>>> strcmp to fix this.
>>>
>>> This fixes a minor issue where internal interfaces are considered
>>> non-internal in the userspace datapath for the purpose of adjusting the
>>> MTU.
>>>
>>> Signed-off-by: Daniele Di Proietto 
>>
>>Acked-by: Thadeu Lima de Souza Cascardo 
>
>Thanks guys, I pushed this to master
>
>>
>>Hi, Mark.
>>
>>Can you keep my Ack in further submissions in case there are no changes to 
>>this
>>patch?


Hi Cascardo,

Apologies - I performed the rework for the entire series on my local branch, so 
your Ack wasn't present.

Thanks to Daniele for re-adding it.

Cheers,
Mark

>>
>>Thanks.
>>Cascardo.
>>
>>> ---
>>>  ofproto/ofproto.c | 16 +---
>>>  1 file changed, 9 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>>> index 8e59c69..088f91a 100644
>>> --- a/ofproto/ofproto.c
>>> +++ b/ofproto/ofproto.c
>>> @@ -220,7 +220,8 @@ static void learned_cookies_flush(struct ofproto *, 
>>> struct ovs_list
>*dead_cookie
>>>  /* ofport. */
>>>  static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
>>>  static void ofport_destroy(struct ofport *, bool del);
>>> -static inline bool ofport_is_internal(const struct ofport *);
>>> +static inline bool ofport_is_internal(const struct ofproto *,
>>> +  const struct ofport *);
>>>
>>>  static int update_port(struct ofproto *, const char *devname);
>>>  static int init_ports(struct ofproto *);
>>> @@ -2465,7 +2466,7 @@ static void
>>>  ofport_remove(struct ofport *ofport)
>>>  {
>>>  struct ofproto *p = ofport->ofproto;
>>> -bool is_internal = ofport_is_internal(ofport);
>>> +bool is_internal = ofport_is_internal(p, ofport);
>>>
>>>  connmgr_send_port_status(ofport->ofproto->connmgr, NULL, >pp,
>>>   OFPPR_DELETE);
>>> @@ -2751,9 +2752,10 @@ init_ports(struct ofproto *p)
>>>  }
>>>
>>>  static inline bool
>>> -ofport_is_internal(const struct ofport *port)
>>> +ofport_is_internal(const struct ofproto *p, const struct ofport *port)
>>>  {
>>> -return !strcmp(netdev_get_type(port->netdev), "internal");
>>> +return !strcmp(netdev_get_type(port->netdev),
>>> +   ofproto_port_open_type(p->type, "internal"));
>>>  }
>>>
>>>  /* Find the minimum MTU of all non-datapath devices attached to 'p'.
>>> @@ -2770,7 +2772,7 @@ find_min_mtu(struct ofproto *p)
>>>
>>>  /* Skip any internal ports, since that's what we're trying to
>>>   * set. */
>>> -if (ofport_is_internal(ofport)) {
>>> +if (ofport_is_internal(p, ofport)) {
>>>  continue;
>>>  }
>>>
>>> @@ -2797,7 +2799,7 @@ update_mtu(struct ofproto *p, struct ofport *port)
>>>  port->mtu = 0;
>>>  return;
>>>  }
>>> -if (ofport_is_internal(port)) {
>>> +if (ofport_is_internal(p, port)) {
>>>  if (dev_mtu > p->min_mtu) {
>>> if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
>>> dev_mtu = p->min_mtu;
>>> @@ -2827,7 +2829,7 @@ update_mtu_ofproto(struct ofproto *p)
>>>  HMAP_FOR_EACH (ofport, hmap_node, >ports) {
>>>  struct netdev *netdev = ofport->netdev;
>>>
>>> -if (ofport_is_internal(ofport)) {
>>> +if (ofport_is_internal(p, ofport)) {
>>>  if (!netdev_set_mtu(netdev, p->min_mtu)) {
>>>  ofport->mtu = p->min_mtu;
>>>  }
>>> --
>>> 1.9.3
>>>
>>> ___
>>> dev mailing list
>>> dev@openvswitch.org
>>> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V7 1/7] ofproto: Consider datapath_type when looking for internal ports.

[Daniele Di Proietto]

On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo"  
wrote:

>On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote:
>> From: Daniele Di Proietto 
>> 
>> Interfaces with type "internal" end up having a netdev with type "tap"
>> in the dpif-netdev datapath, so a strcmp will fail to match internal
>> interfaces.
>> 
>> We can translate the types with ofproto_port_open_type() before calling
>> strcmp to fix this.
>> 
>> This fixes a minor issue where internal interfaces are considered
>> non-internal in the userspace datapath for the purpose of adjusting the
>> MTU.
>> 
>> Signed-off-by: Daniele Di Proietto 
>
>Acked-by: Thadeu Lima de Souza Cascardo 

Thanks guys, I pushed this to master

>
>Hi, Mark.
>
>Can you keep my Ack in further submissions in case there are no changes to this
>patch?
>
>Thanks.
>Cascardo.
>
>> ---
>>  ofproto/ofproto.c | 16 +---
>>  1 file changed, 9 insertions(+), 7 deletions(-)
>> 
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index 8e59c69..088f91a 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -220,7 +220,8 @@ static void learned_cookies_flush(struct ofproto *, 
>> struct ovs_list *dead_cookie
>>  /* ofport. */
>>  static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
>>  static void ofport_destroy(struct ofport *, bool del);
>> -static inline bool ofport_is_internal(const struct ofport *);
>> +static inline bool ofport_is_internal(const struct ofproto *,
>> +  const struct ofport *);
>>  
>>  static int update_port(struct ofproto *, const char *devname);
>>  static int init_ports(struct ofproto *);
>> @@ -2465,7 +2466,7 @@ static void
>>  ofport_remove(struct ofport *ofport)
>>  {
>>  struct ofproto *p = ofport->ofproto;
>> -bool is_internal = ofport_is_internal(ofport);
>> +bool is_internal = ofport_is_internal(p, ofport);
>>  
>>  connmgr_send_port_status(ofport->ofproto->connmgr, NULL, >pp,
>>   OFPPR_DELETE);
>> @@ -2751,9 +2752,10 @@ init_ports(struct ofproto *p)
>>  }
>>  
>>  static inline bool
>> -ofport_is_internal(const struct ofport *port)
>> +ofport_is_internal(const struct ofproto *p, const struct ofport *port)
>>  {
>> -return !strcmp(netdev_get_type(port->netdev), "internal");
>> +return !strcmp(netdev_get_type(port->netdev),
>> +   ofproto_port_open_type(p->type, "internal"));
>>  }
>>  
>>  /* Find the minimum MTU of all non-datapath devices attached to 'p'.
>> @@ -2770,7 +2772,7 @@ find_min_mtu(struct ofproto *p)
>>  
>>  /* Skip any internal ports, since that's what we're trying to
>>   * set. */
>> -if (ofport_is_internal(ofport)) {
>> +if (ofport_is_internal(p, ofport)) {
>>  continue;
>>  }
>>  
>> @@ -2797,7 +2799,7 @@ update_mtu(struct ofproto *p, struct ofport *port)
>>  port->mtu = 0;
>>  return;
>>  }
>> -if (ofport_is_internal(port)) {
>> +if (ofport_is_internal(p, port)) {
>>  if (dev_mtu > p->min_mtu) {
>> if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
>> dev_mtu = p->min_mtu;
>> @@ -2827,7 +2829,7 @@ update_mtu_ofproto(struct ofproto *p)
>>  HMAP_FOR_EACH (ofport, hmap_node, >ports) {
>>  struct netdev *netdev = ofport->netdev;
>>  
>> -if (ofport_is_internal(ofport)) {
>> +if (ofport_is_internal(p, ofport)) {
>>  if (!netdev_set_mtu(netdev, p->min_mtu)) {
>>  ofport->mtu = p->min_mtu;
>>  }
>> -- 
>> 1.9.3
>> 
>> ___
>> dev mailing list
>> dev@openvswitch.org
>> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH V7 1/7] ofproto: Consider datapath_type when looking for internal ports.

[Thadeu Lima de Souza Cascardo]

On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote:
> From: Daniele Di Proietto 
> 
> Interfaces with type "internal" end up having a netdev with type "tap"
> in the dpif-netdev datapath, so a strcmp will fail to match internal
> interfaces.
> 
> We can translate the types with ofproto_port_open_type() before calling
> strcmp to fix this.
> 
> This fixes a minor issue where internal interfaces are considered
> non-internal in the userspace datapath for the purpose of adjusting the
> MTU.
> 
> Signed-off-by: Daniele Di Proietto 

Acked-by: Thadeu Lima de Souza Cascardo 

Hi, Mark.

Can you keep my Ack in further submissions in case there are no changes to this
patch?

Thanks.
Cascardo.

> ---
>  ofproto/ofproto.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 8e59c69..088f91a 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -220,7 +220,8 @@ static void learned_cookies_flush(struct ofproto *, 
> struct ovs_list *dead_cookie
>  /* ofport. */
>  static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
>  static void ofport_destroy(struct ofport *, bool del);
> -static inline bool ofport_is_internal(const struct ofport *);
> +static inline bool ofport_is_internal(const struct ofproto *,
> +  const struct ofport *);
>  
>  static int update_port(struct ofproto *, const char *devname);
>  static int init_ports(struct ofproto *);
> @@ -2465,7 +2466,7 @@ static void
>  ofport_remove(struct ofport *ofport)
>  {
>  struct ofproto *p = ofport->ofproto;
> -bool is_internal = ofport_is_internal(ofport);
> +bool is_internal = ofport_is_internal(p, ofport);
>  
>  connmgr_send_port_status(ofport->ofproto->connmgr, NULL, >pp,
>   OFPPR_DELETE);
> @@ -2751,9 +2752,10 @@ init_ports(struct ofproto *p)
>  }
>  
>  static inline bool
> -ofport_is_internal(const struct ofport *port)
> +ofport_is_internal(const struct ofproto *p, const struct ofport *port)
>  {
> -return !strcmp(netdev_get_type(port->netdev), "internal");
> +return !strcmp(netdev_get_type(port->netdev),
> +   ofproto_port_open_type(p->type, "internal"));
>  }
>  
>  /* Find the minimum MTU of all non-datapath devices attached to 'p'.
> @@ -2770,7 +2772,7 @@ find_min_mtu(struct ofproto *p)
>  
>  /* Skip any internal ports, since that's what we're trying to
>   * set. */
> -if (ofport_is_internal(ofport)) {
> +if (ofport_is_internal(p, ofport)) {
>  continue;
>  }
>  
> @@ -2797,7 +2799,7 @@ update_mtu(struct ofproto *p, struct ofport *port)
>  port->mtu = 0;
>  return;
>  }
> -if (ofport_is_internal(port)) {
> +if (ofport_is_internal(p, port)) {
>  if (dev_mtu > p->min_mtu) {
> if (!netdev_set_mtu(port->netdev, p->min_mtu)) {
> dev_mtu = p->min_mtu;
> @@ -2827,7 +2829,7 @@ update_mtu_ofproto(struct ofproto *p)
>  HMAP_FOR_EACH (ofport, hmap_node, >ports) {
>  struct netdev *netdev = ofport->netdev;
>  
> -if (ofport_is_internal(ofport)) {
> +if (ofport_is_internal(p, ofport)) {
>  if (!netdev_set_mtu(netdev, p->min_mtu)) {
>  ofport->mtu = p->min_mtu;
>  }
> -- 
> 1.9.3
> 
> ___
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev