Regards
_Sugesh


> -----Original Message-----
> From: Roi Dayan [mailto:[email protected]]
> Sent: Wednesday, February 15, 2017 9:25 AM
> To: Chandran, Sugesh <[email protected]>; [email protected]
> Cc: [email protected]; Paul Blakey <[email protected]>; Or Gerlitz
> <[email protected]>; Hadar Hen Zion <[email protected]>; Shahar
> Klein <[email protected]>; Mark Bloch <[email protected]>; Rony
> Efraim <[email protected]>; Fastabend, John R
> <[email protected]>; Joe Stringer <[email protected]>; Andy
> Gospodarek <[email protected]>; Lance Richardson
> <[email protected]>; Marcelo Ricardo Leitner <[email protected]>;
> Simon Horman <[email protected]>; Jiri Pirko
> <[email protected]>
> Subject: Re: [PATCH ovs V3 10/25] netdev-tc-offloads: Add ufid to tc/netdev
> map
> 
> 
> 
> On 14/02/2017 01:54, Chandran, Sugesh wrote:
> >
> >
> > Regards
> > _Sugesh
> >
> >
> >> -----Original Message-----
> >> From: Roi Dayan [mailto:[email protected]]
> >> Sent: Wednesday, February 8, 2017 3:29 PM
> >> To: [email protected]
> >> Cc: Paul Blakey <[email protected]>; Or Gerlitz
> >> <[email protected]>; Hadar Hen Zion <[email protected]>;
> Shahar
> >> Klein <[email protected]>; Mark Bloch <[email protected]>;
> Rony
> >> Efraim <[email protected]>; Fastabend, John R
> >> <[email protected]>; Joe Stringer <[email protected]>; Andy
> >> Gospodarek <[email protected]>; Lance Richardson
> >> <[email protected]>; Marcelo Ricardo Leitner
> <[email protected]>;
> >> Simon Horman <[email protected]>; Jiri Pirko
> >> <[email protected]>; Chandran, Sugesh <[email protected]>
> >> Subject: [PATCH ovs V3 10/25] netdev-tc-offloads: Add ufid to
> >> tc/netdev map
> >
> > .....
> >> +
> > [Sugesh] Good to have comment on each functions explaining about the
> return values and parameters?
> 
> ok
> 
> >> +static bool
> >> +del_ufid_tc_mapping(const ovs_u128 *ufid) {
> >> +    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
> >> +    struct ufid_to_tc_data *data;
> >> +
> >> +    ovs_mutex_lock(&ufid_lock);
> >> +    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &ufid_to_tc) {
> >> +        if (ovs_u128_equals(*ufid, data->ufid)) {
> >> +            break;
> >> +        }
> >> +    }
> >> +    if (data) {
> >> +        hmap_remove(&ufid_to_tc, &data->node);
> >> +        ovs_mutex_unlock(&ufid_lock);
> >> +        netdev_close(data->netdev);
> >> +        free(data);
> >> +        return true;
> >> +    }
> >> +    ovs_mutex_unlock(&ufid_lock);
> >> +    return false;
> >> +}
> >> +
> >> +static ovs_u128 *
> >> +find_ufid(int prio, int handle, struct netdev *netdev) {
> >> +    int ifindex = netdev_get_ifindex(netdev);
> >> +    struct ufid_to_tc_data *data;
> >> +
> >> +    ovs_mutex_lock(&ufid_lock);
> >> +    HMAP_FOR_EACH(data, node, &ufid_to_tc) {
> >> +        if (data->prio == prio && data->handle == handle
> >> +            && netdev_get_ifindex(data->netdev) == ifindex) {
> >> +            break;
> >> +        }
> >> +    }
> >> +    ovs_mutex_unlock(&ufid_lock);
> >> +    if (data) {
> >> +        return &data->ufid;
> >> +    }
> >> +    return NULL;
> >> +}
> >> +
> >> +static int
> >> +get_ufid_tc_mapping(const ovs_u128 *ufid, int *prio, struct netdev
> >> +**netdev) {
> >> +    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
> >> +    struct ufid_to_tc_data *data;
> >> +
> >> +    ovs_mutex_lock(&ufid_lock);
> >> +    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &ufid_to_tc) {
> >> +        if (ovs_u128_equals(*ufid, data->ufid)) {
> >> +            break;
> >> +        }
> >> +    }
> >> +    ovs_mutex_unlock(&ufid_lock);
> >> +    if (data) {
> >> +        if (prio) {
> >> +            *prio = data->prio;
> >> +        }
> >> +        if (netdev) {
> >> +            *netdev = netdev_ref(data->netdev);
> >> +        }
> >> +        return data->handle;
> >> +    }
> >> +    return 0;
> >> +}
> >> +
> > [Sugesh] I assume its add +replace than just add? May be its good to add
> comment or changing the function name accordingly.
> 
> ok
> I'll also reorder a little here. add/del/get/find/
[Sugesh] thanks!
> 
> >> +static bool
> >> +add_ufid_tc_mapping(const ovs_u128 *ufid, int prio, int handle,
> >> +                    struct netdev *netdev) {
> >> +    size_t hash = hash_bytes(ufid, sizeof *ufid, 0);
> >> +    bool replace = del_ufid_tc_mapping(ufid);
> >> +    struct ufid_to_tc_data *new_data = xzalloc(sizeof *new_data);
> >> +
> >> +    new_data->ufid = *ufid;
> >> +    new_data->prio = prio;
> >> +    new_data->handle = handle;
> >> +    new_data->netdev = netdev_ref(netdev);
> >> +
> >> +    ovs_mutex_lock(&ufid_lock);
> >> +    hmap_insert(&ufid_to_tc, &new_data->node, hash);
> >> +    ovs_mutex_unlock(&ufid_lock);
> >> +
> >> +    return replace;
> >> +}
> >> +
> >>  int
> >>  netdev_tc_flow_flush(struct netdev *netdev)  {
> >> --
> >> 2.7.4
> >
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to