netdev - Re: [ofa-general] Re: [PATCH 01/13] QLogic VNIC: Driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 5 May 2008 21:06:58 +0530
From:	"Ramachandra K" <ramachandra.kuchimanchi@...gic.com>
To:	"Roland Dreier" <rdreier@...co.com>
Cc:	netdev@...r.kernel.org, poornima.kamath@...gic.com,
	general@...ts.openfabrics.org, amar.mudrankit@...gic.com
Subject: Re: [ofa-general] Re: [PATCH 01/13] QLogic VNIC: Driver - netdev implementation

Roland,

Thanks for the review. Your comments make sense and we will fix the
things you pointed out. Please see some clarifications in-line.

On Fri, May 2, 2008 at 11:45 PM, Roland Dreier <rdreier@...co.com> wrote:
>  > From: Ramachandra K <ramachandra.kuchimanchi@...gic.com>
>   >
>   > Signed-off-by: Poornima Kamath <poornima.kamath@...gic.com>
>   > Signed-off-by: Amar Mudrankit <amar.mudrankit@...gic.com>
>
>  For the next submission please clean up the From and Signed-off-by
>  lines.  As it stands now you are saying that you (Ramachandra K) are the
>  author of the patch, and that Poornima and Amar signed off on it (ie
>  forwarded it), but you as the person sending the email did not sign off
>  on it.
>

I will make sure to sign off on all patches. Should I also drop the From line
for the patches which I developed, since I am mailing them myself ?

I am using the Signed-off-by line to indicate the people who were
involved in the development of the patches at some stage.

>
>
>   > +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);
>   > +    }
>   > +}
>
>  Do you have sufficient locking here?  Could vnic->current_path or
>  vnic->xmit_started change after they are tested, leading to bad results?
>  Also do you get anything from having a xmit_started flag that you
>  couldn't get just by testing with netif_queue_stopped()?
>

You are right, xmit_started might not be required and we will look at
the locking
issue too.

>
>
>   > +extern cycles_t recv_ref;
>
>  seems like too generic a name to make global.  What the heck are you
>  using cycle_t to keep track of anyway?
>

This is being used as part of the driver internal statistics
collection to keep track of the time
elapsed between a message arriving from the EVIC indicating that it
has done an RDMA write of
an Ethernet packet to the driver memory and the driver giving the packet
to the network stack.  Will fix the variable name.

Regards,
Ram
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html