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

Reply via email to