[ovs-dev] [PATCH] ofproto-dpif.c: Replace vsp_realdev_to_vlandev() with two new functions

Ethan Jackson ethan at nicira.com
Wed May 29 22:46:08 UTC 2013


> I'm not sure how likely the function "vsp_realdev_to_vlandev()" will be
> called with odp_port as argument in the future. I'll go with the "two
> functions" implementation first.

FWIW I suspect this is unlikely, and even if it wasn't.  We can always
change it in future when we have more information about what's really
necessary.

Ethan


> Hope to hear more comments from you.
>
> Thanks
>
>
>
> On Wed, May 29, 2013 at 3:25 PM, Ethan Jackson <ethan at nicira.com> wrote:
>>
>> 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 <alexw at 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 <alexw at 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 at openvswitch.org
>> > http://openvswitch.org/mailman/listinfo/dev
>
>



More information about the dev mailing list