I haven't looked at this patch carefully, so my next comment may or may not be worthwhile, but I'm wondering if it would be cleaner if we kept a single vsp_realdev_to_vlandev, which took an ofp_port as an argument, and then had compose_output_action__ pass in the openflow port number instead of the datapath port number to that function?
Ethan On Wed, May 29, 2013 at 5:01 PM, Alex Wang <al...@nicira.com> wrote: > In ofproto-dpif.c, function vsp_realdev_to_vlandev() is called with both > OpenFlow port and datapath port number as argument. This patch replaces > the function vsp_realdev_to_vlandev() with vsp_ofp_realdev_to_vlandev() and > vsp_odp_realdev_to_vlandev(), which take OpenFlow port number and datapth > port number respectively. > > Signed-off-by: Alex Wang <al...@nicira.com> > --- > ofproto/ofproto-dpif.c | 61 > +++++++++++++++++++++++++++++++++--------------- > 1 file changed, 42 insertions(+), 19 deletions(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 280fd57..193064b 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -586,8 +586,12 @@ struct vlan_splinter { > int vid; > }; > > -static uint32_t vsp_realdev_to_vlandev(const struct ofproto_dpif *, > - uint32_t realdev, ovs_be16 vlan_tci); > +static uint16_t vsp_ofp_realdev_to_vlandev(const struct ofproto_dpif *, > + uint16_t realdev, > + ovs_be16 vlan_tci); > +static uint32_t vsp_odp_realdev_to_vlandev(const struct ofproto_dpif *, > + uint32_t realdev, > + ovs_be16 vlan_tci); > static bool vsp_adjust_flow(const struct ofproto_dpif *, struct flow *); > static void vsp_remove(struct ofport_dpif *); > static void vsp_add(struct ofport_dpif *, uint16_t realdev_ofp_port, int > vid); > @@ -6183,8 +6187,8 @@ compose_output_action__(struct action_xlate_ctx *ctx, > uint16_t ofp_port, > ctx->flow.tunnel = flow_tnl; /* Restore tunnel metadata */ > } else { > odp_port = ofport->odp_port; > - out_port = vsp_realdev_to_vlandev(ctx->ofproto, odp_port, > - ctx->flow.vlan_tci); > + out_port = vsp_odp_realdev_to_vlandev(ctx->ofproto, odp_port, > + ctx->flow.vlan_tci); > if (out_port != odp_port) { > ctx->flow.vlan_tci = htons(0); > } > @@ -8729,33 +8733,52 @@ hash_realdev_vid(uint16_t realdev_ofp_port, int vid) > return hash_2words(realdev_ofp_port, vid); > } > > -/* Returns the ODP port number of the Linux VLAN device that corresponds to > - * 'vlan_tci' on the network device with port number 'realdev_odp_port' in > - * 'ofproto'. For example, given 'realdev_odp_port' of eth0 and 'vlan_tci' > 9, > - * it would return the port number of eth0.9. > +/* Returns the OFP port number of the Linux VLAN device that corresponds to > + * 'vlan_tci' on the network device with port number 'realdev_ofp_port' in > + * 'ofport_dpif'. For example, given 'realdev_ofp_port' of eth0 and > + * 'vlan_tci' 9, it would return the port number of eth0.9. > * > - * Unless VLAN splinters are enabled for port 'realdev_odp_port', this > - * function just returns its 'realdev_odp_port' argument. */ > -static uint32_t > -vsp_realdev_to_vlandev(const struct ofproto_dpif *ofproto, > - uint32_t realdev_odp_port, ovs_be16 vlan_tci) > + * Unless VLAN splinters are enabled for port 'realdev_ofp_port', this > + * function just returns its 'realdev_ofp_port' argument. */ > +static uint16_t > +vsp_ofp_realdev_to_vlandev(const struct ofproto_dpif *ofproto, > + uint16_t realdev_ofp_port, ovs_be16 vlan_tci) > { > if (!hmap_is_empty(&ofproto->realdev_vid_map)) { > - uint16_t realdev_ofp_port; > int vid = vlan_tci_to_vid(vlan_tci); > const struct vlan_splinter *vsp; > > - realdev_ofp_port = odp_port_to_ofp_port(ofproto, realdev_odp_port); > HMAP_FOR_EACH_WITH_HASH (vsp, realdev_vid_node, > hash_realdev_vid(realdev_ofp_port, vid), > &ofproto->realdev_vid_map) { > if (vsp->realdev_ofp_port == realdev_ofp_port > && vsp->vid == vid) { > - return ofp_port_to_odp_port(ofproto, vsp->vlandev_ofp_port); > + return vsp->vlandev_ofp_port; > } > } > } > - return realdev_odp_port; > + return realdev_ofp_port; > +} > + > +/* Returns the ODP port number of the Linux VLAN device that corresponds to > + * 'vlan_tci' on the network device with datapath port number > + * 'realdev_odp_port'. For example, given 'realdev_odp_port' of eth0 and > + * 'vlan_tci' 9, it would return the port number of eth0.9. > + * > + * Unless VLAN splinters are enabled for port 'realdev_odp_port', this > + * function just returns its 'realdev_odp_port' argument. */ > +static uint32_t > +vsp_odp_realdev_to_vlandev(const struct ofproto_dpif *ofproto, > + uint32_t realdev_odp_port, ovs_be16 vlan_tci) > +{ > + uint16_t realdev_ofp_port; > + uint16_t vlandev_ofp_port; > + > + realdev_ofp_port = odp_port_to_ofp_port(ofproto, realdev_odp_port); > + vlandev_ofp_port = vsp_ofp_realdev_to_vlandev(ofproto, realdev_ofp_port, > + vlan_tci); > + > + return ofp_port_to_odp_port(ofproto, vlandev_ofp_port); > } > > static struct vlan_splinter * > @@ -8848,8 +8871,8 @@ vsp_add(struct ofport_dpif *port, uint16_t > realdev_ofp_port, int vid) > struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto); > > if (!vsp_vlandev_to_realdev(ofproto, port->up.ofp_port, NULL) > - && (vsp_realdev_to_vlandev(ofproto, realdev_ofp_port, htons(vid)) > - == realdev_ofp_port)) { > + && (vsp_ofp_realdev_to_vlandev(ofproto, realdev_ofp_port, htons(vid)) > + == realdev_ofp_port)) { > struct vlan_splinter *vsp; > > vsp = xmalloc(sizeof *vsp); > -- > 1.7.9.5 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev