* about remus and xl patches
@ 2013-12-16 7:04 wencongyang
2013-12-16 7:57 ` Shriram Rajagopalan
0 siblings, 1 reply; 4+ messages in thread
From: wencongyang @ 2013-12-16 7:04 UTC (permalink / raw)
To: rshriram, xen-devel, eddie.dong
hi, shriram
i don't use this mail to subscribe to the list. so i don't reply the thread.
1. about the script, you check some modules, but the user can build it into the kernel....
2. in the function init_qdisc(), you use nl_socket_alloc() to alloc new netlink socket, but i don't find nl_socket_free()
. You don't cleanup when something fails?
3. in the function get_guest_vif_list(), you call get_vifname() in LOG(), you don't free the memory.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: about remus and xl patches
2013-12-16 7:04 about remus and xl patches wencongyang
@ 2013-12-16 7:57 ` Shriram Rajagopalan
2013-12-16 10:02 ` Ian Campbell
2013-12-18 1:52 ` Wen Congyang
0 siblings, 2 replies; 4+ messages in thread
From: Shriram Rajagopalan @ 2013-12-16 7:57 UTC (permalink / raw)
To: wencongyang; +Cc: eddie.dong, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 1383 bytes --]
On Mon, Dec 16, 2013 at 1:04 AM, wencongyang <wencongyang@gmail.com> wrote:
> hi, shriram
>
> i don't use this mail to subscribe to the list. so i don't reply the
> thread.
>
> 1. about the script, you check some modules, but the user can build it
> into the kernel....
>
At the moment, I am only concerned about users who just want to run Remus
on their stock distribution.
(Over the last two years, a majority of Remus help requests I have received
were from people who didnt have the expertise
or did not want to mess with custom kernels on their test boxes :) ). So I
figured if someone is smart enough to compile
a module into the kernel itself, he/she would be easily able to fix the
script to make Remus work in their setup :).
> 2. in the function init_qdisc(), you use nl_socket_alloc() to alloc new
> netlink socket, but i don't find nl_socket_free()
>
Good catch. Thanks!
> . You don't cleanup when something fails?
>
I do. Check the *_teardown functions. It currently uses nl_close. But I
should be using nl_socket_free as it seems to invoke
nl_close too.
> 3. in the function get_guest_vif_list(), you call get_vifname() in LOG(),
> you don't free the memory.
>
>
>
No need to, AFAIK. get_vifname allocates the string in gc context.
Besides, get_vif_list (and subsequently get_vifname) is a
one time call during net buffer setup, i.e., before Remus starts.
[-- Attachment #1.2: Type: text/html, Size: 2305 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: about remus and xl patches
2013-12-16 7:57 ` Shriram Rajagopalan
@ 2013-12-16 10:02 ` Ian Campbell
2013-12-18 1:52 ` Wen Congyang
1 sibling, 0 replies; 4+ messages in thread
From: Ian Campbell @ 2013-12-16 10:02 UTC (permalink / raw)
To: rshriram; +Cc: wencongyang, eddie.dong, xen-devel
On Mon, 2013-12-16 at 01:57 -0600, Shriram Rajagopalan wrote:
> On Mon, Dec 16, 2013 at 1:04 AM, wencongyang <wencongyang@gmail.com>
> wrote:
> hi, shriram
>
> i don't use this mail to subscribe to the list. so i don't
> reply the thread.
>
> 1. about the script, you check some modules, but the user can
> build it into the kernel....
>
>
> At the moment, I am only concerned about users who just want to run
> Remus on their stock distribution.
> (Over the last two years, a majority of Remus help requests I have
> received were from people who didnt have the expertise
> or did not want to mess with custom kernels on their test boxes :) ).
> So I figured if someone is smart enough to compile
> a module into the kernel itself, he/she would be easily able to fix
> the script to make Remus work in their setup :).
FWIW rather than using modinfo I think the normal approach is to use
modprobe [-q] which IIRC does the right thing if the module is
statically compiled in.
Ian.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: about remus and xl patches
2013-12-16 7:57 ` Shriram Rajagopalan
2013-12-16 10:02 ` Ian Campbell
@ 2013-12-18 1:52 ` Wen Congyang
1 sibling, 0 replies; 4+ messages in thread
From: Wen Congyang @ 2013-12-18 1:52 UTC (permalink / raw)
To: rshriram; +Cc: wencongyang, eddie.dong, xen-devel
At 12/16/2013 03:57 PM, Shriram Rajagopalan Wrote:
> On Mon, Dec 16, 2013 at 1:04 AM, wencongyang <wencongyang@gmail.com> wrote:
>
>> hi, shriram
>>
>> i don't use this mail to subscribe to the list. so i don't reply the
>> thread.
>>
>> 1. about the script, you check some modules, but the user can build it
>> into the kernel....
>>
>
> At the moment, I am only concerned about users who just want to run Remus
> on their stock distribution.
> (Over the last two years, a majority of Remus help requests I have received
> were from people who didnt have the expertise
> or did not want to mess with custom kernels on their test boxes :) ). So I
> figured if someone is smart enough to compile
> a module into the kernel itself, he/she would be easily able to fix the
> script to make Remus work in their setup :).
sch_plug sch_ingress act_mirred cls_u32 will be loaded automatically by tc commands.
So I think there is no need to check them. If these modules doesn't exist
or are built into kernel, tc will fail, and we can report error later.
Thanks
Wen Congyang
>
>
>
>> 2. in the function init_qdisc(), you use nl_socket_alloc() to alloc new
>> netlink socket, but i don't find nl_socket_free()
>>
>
> Good catch. Thanks!
>
>
>> . You don't cleanup when something fails?
>>
>
> I do. Check the *_teardown functions. It currently uses nl_close. But I
> should be using nl_socket_free as it seems to invoke
> nl_close too.
>
>
>> 3. in the function get_guest_vif_list(), you call get_vifname() in LOG(),
>> you don't free the memory.
>>
>>
>>
> No need to, AFAIK. get_vifname allocates the string in gc context.
> Besides, get_vif_list (and subsequently get_vifname) is a
> one time call during net buffer setup, i.e., before Remus starts.
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-12-18 1:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 7:04 about remus and xl patches wencongyang
2013-12-16 7:57 ` Shriram Rajagopalan
2013-12-16 10:02 ` Ian Campbell
2013-12-18 1:52 ` Wen Congyang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.