| Message ID | 1470758480-133197-1-git-send-email-mark.b.kavanagh@intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote: > From: Daniele Di Proietto <diproiettod@vmware.com> > > 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 <diproiettod@vmware.com> Acked-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> 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, &ofport->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, &p->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
On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo" <cascardo@redhat.com> wrote: >On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote: >> From: Daniele Di Proietto <diproiettod@vmware.com> >> >> 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 <diproiettod@vmware.com> > >Acked-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> 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, &ofport->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, &p->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
> >On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo" <cascardo@redhat.com> wrote: > >>On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote: >>> From: Daniele Di Proietto <diproiettod@vmware.com> >>> >>> 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 <diproiettod@vmware.com> >> >>Acked-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > >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, &ofport->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, &p->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
On Wed, Aug 10, 2016 at 10:09:07AM +0000, Kavanagh, Mark B wrote: > > > >On 09/08/2016 09:08, "Thadeu Lima de Souza Cascardo" <cascardo@redhat.com> wrote: > > > >>On Tue, Aug 09, 2016 at 05:01:14PM +0100, Mark Kavanagh wrote: > >>> From: Daniele Di Proietto <diproiettod@vmware.com> > >>> > >>> 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 <diproiettod@vmware.com> > >> > >>Acked-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com> > > > >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.
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, &ofport->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, &p->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; }