On 7/28/10 1:39 PM, Chris Leech wrote:
> On Wed, Jul 28, 2010 at 11:46:24AM -0700, Joe Eykholt wrote:
>> On 7/28/10 11:42 AM, Chris Leech wrote:
>>> If the link state is changing while fipvlan is running, it could result in a
>>> packet socket being added to the poll list more than once.  When that 
>>> happened,
>>> recvmsg would be called multiple times for a single FIP response.
>>>
>>> Previously this caused a hang, which the change to MSG_DONTWAIT worked 
>>> around,
>>> but an error message is still printed every time the condition occurs.  This
>>> change fixes the poll list managment to correctly avoid the error.
>>>
>>> Signed-off-by: Chris Leech<[email protected]>
>>> ---
>>>
>>>    fipvlan.c |    7 +++++++
>>>    1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fipvlan.c b/fipvlan.c
>>> index eddffbb..b400427 100644
>>> --- a/fipvlan.c
>>> +++ b/fipvlan.c
>>> @@ -80,6 +80,11 @@ static int pfd_len = 0;
>>>    void pfd_add(int fd)
>>>    {
>>>     struct pollfd *npfd;
>>> +   int i;
>>> +
>>> +   for (i = 0; i<   pfd_len; i++)
>>> +           if (pfd[i].fd == fd)
>>> +                   return;
>>>
>>>     npfd = realloc(pfd, (pfd_len + 1) * sizeof(struct pollfd));
>>>     if (!npfd) {
>>> @@ -319,6 +324,8 @@ void rtnl_recv_newlink(struct nlmsghdr *nh)
>>>             iff->running = (ifm->ifi_flags&   IFF_RUNNING) == IFF_RUNNING;
>>>             if (iff->running)
>>>                     pfd_add(iff->ps);
>>> +           else
>>> +                   pfd_remove(iff->ps);
>>>             return;
>>>     }
>>>
>>>
>>
>> Is it possible the caller of pfd_add would later want to remove the socket
>> from the list and succeed even though it wasn't the last user?
>>
>> I didn't look into this, but thought I'd mention the issue anyway.
>> Maybe it can't happen.
>
> Or (I just re-read an interpreted your email differently), or you asking
> about the pfd_add return added here preventing what would essentially be
> an attempt to reference count the fd?

That's what I was wondering.  If someone did two pfd_adds, then one pfd_remove,
all for the same fd, they might not expect it to work that way, but I didn't
see if anyone would do that.

> I'll admit that the two chunks of this patch are both addressing parts
> of same bug in different ways; it's both removing the fd on link down
> and making sure that if there are multiple netlink change events that
> all show link up the fd will only be added once.

OK, I see.  That seems safe.

        Cheers,
        Joe

_______________________________________________
devel mailing list
[email protected]
http://www.open-fcoe.org/mailman/listinfo/devel

Reply via email to