Switch stat description does not conform to the specification

[ZoltanLajosKis]

Hi All,

I have a question regarding sending back total DESC_STR_LEN (256) bytes for mfr_desc and rest of the switch description data.
Why is pointer "stats->mfr_desc" (and rest in the structure) in function ofl_msg_pack_stats_reply_desc( ) in file ofl-messages-pack.c not initialized with 0 and then why it doesn't copy from msg->mfr_desc until it receives '\0' instead of copying whole DESC_STR_LEN bytes?

Thanks,
Arun

---

Hi Arun,

You are right, this does not conform to the specification, as that requires those strings to be padded with zeroes.
It should cause no harm, as the fields will contain a zero delimited string (although it might freak out some memory managers :-).

Kind Regards,
Zoltan.

[Arun-Paneri]

Well, I am more concern about sending extra bytes out of the switch. In my switch msg->mfr_desc contains hardly 20bytes of data but OFL sending 235 extra bytes back to the controller.

[ZoltanLajosKis]

The specification requires the switch to send out 256 bytes for the description (padded if string is shorter). This code does that. If you want the specification to be changed, you should file a ticket to ONF/ExtWG about that.

Also, the switch features description is sent out only once per OpenFlow session. Saving 235 bytes per session is pointless I would say.

[Arun-Paneri]

You are right, saving extra bytes is pointless but sending extra information (may be a garbage ) may contain something that controller/admin is not suppose to see.
So the solution is, in "stats->mfr_desc" copy only the bytes "msg->mfr_desc" actually wants OFL to copy and let the rest padded with 0.
So I am not against sending 256 bytes but sending extra information which switch doesnt want to send.

Thanks.

[ZoltanLajosKis]

OK, now I understand what you meant in the previous comment. So if the padding is implemented, this concern is fixed as well.

[Arun-Paneri]

One more thing, I have only 20bytes of mfr description and so I SHOULD malloc only 20bytes in "msg->mfr_desc". But when OFL copy "msg->mfr_desc" in "stats->mfr_desc", it goes ahead and copy beyond 20bytes. Now if OFL also copy only 20bytes(till finds '\0') then I need not to malloc whole 256byes for mfr_desc. Though again keeping 1056 bytes total (4*256 + 32 for serial_num) for structure ofp_desc_stats is not a big deal.
Thanks.

[chesteve]

If anyone issues a pull request for this issue we are happy to accept it
asap, otherwise we will certainly look at the issue and provide a fix, but
it wont be as fast as there are other pending tasks on the queue.

Thx,
Christian

On Sat, Feb 16, 2013 at 2:51 PM, Arunkjdhsfkhsdfhjsdf <
notifications@github.com> wrote:

One more thing, I have only 20bytes of mfr description and so I SHOULD
malloc only 20bytes in "msg->mfr_desc". But when OFL copy "msg->mfr_desc"
in "stats->mfr_desc", it goes ahead and copy beyond 20bytes. Now if OFL
also copy only 20bytes(till finds '\0') then I need not to malloc whole
256byes for mfr_desc. Though again keeping 1056 bytes total (4*256 + 32 for
serial_num) for structure ofp_desc_stats is not a big deal.
Thanks.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/14#issuecomment-13658060.

Christian Esteve Rothenberg, Ph.D.
Converged Networks Business Unit
CPqD - Center for Research and Development in Telecommunications
Tel. (+55 19) 3705 4479 / Cel. (+55 19) 8193-7087

[ederlf]

Fixed.