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