Ramachandra K wrote:
> +void vnic_connected(struct vnic *vnic, struct netpath *netpath)
> +{
> + VNIC_FUNCTION("vnic_connected()\n");
> + if (netpath->second_bias)
> + vnic_npevent_queue_evt(netpath, VNIC_SECNP_CONNECTED);
> + else
> + vnic_npevent_queue_evt(netpath, VNIC_PRINP_CONNECTED);
> +
> + vnic_connected_stats(vnic);
> +}
This driver looks rather like it's duplicating at least some of the
functionality provided by the bonding driver. Could you explain the
differences between the two approaches, please?
> +void vnic_stop_xmit(struct vnic *vnic, struct netpath *netpath)
> +{
> + VNIC_FUNCTION("vnic_stop_xmit()\n");
> + if (netpath == vnic->current_path) {
> + if (vnic->xmit_started) {
> + netif_stop_queue(&vnic->netdevice);
> + vnic->xmit_started = 0;
> + }
> +
> + vnic_stop_xmit_stats(vnic);
> + }
> +}
Why is there an asymmetry between some of the routines, which will
operate on either netpath depending on the bias setting, and others like
this, which only operate on the current netpath and silently do nothing
otherwise? This is at least confusing.
> +static struct vnic * vnic_handle_npevent(struct vnic *vnic,
> + enum vnic_npevent_type npevt_type)
> +{
> + struct netpath *netpath;
> +
> + VNIC_INFO("%s: processing %s, netpath=%s, carrier=%d\n",
> + vnic->config->name, vnic_npevent_str[npevt_type],
> + netpath_to_string(vnic, vnic->current_path),
> + vnic->carrier);
> +
> + switch (npevt_type) {
> + case VNIC_PRINP_CONNECTED:
I still don't understand this business of queueing events in the normal
net driver entry points, then redispatching them in this big switch
statement. Could you at least add a comment to the code that indicates
why you're doing this? This structure makes the driver harder to follow
without providing any obvious benefits.
<b
_______________________________________________
openib-general mailing list
[email protected]
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
