Page MenuHomeVyOS Platform

Some bugfixes of vyatta-wanloadbalance
Closed, ResolvedPublicBUG

Description

  1. Error on exit when clear routing tables
  2. Possible crash (buffer overflow) in function for receive icmp echo packet
  3. Error in parsing of config file for udp ttl "probe"
  4. Wrong error messge in analyzing config file
diff -Nurp vyatta-wanloadbalance-0.13.70-old/src/lbdecision.cc vyatta-wanloadbalance-0.13.70/src/lbdecision.cc
--- vyatta-wanloadbalance-0.13.70-old/src/lbdecision.cc	2020-05-15 17:33:53.000000000 +0300
+++ vyatta-wanloadbalance-0.13.70/src/lbdecision.cc	2020-07-07 15:02:50.914265756 +0300
@@ -488,7 +488,7 @@ LBDecision::shutdown(LBData &data)
     sprintf(buf,"%d",h_iter->second._interface_index + IPT_MARK_OFFSET);
     
     execute(string("ip rule del table ") + buf, stdout);
-    execute(string("ip route del table ") + buf, stdout);
+    execute(string("ip route flush table ") + buf, stdout);
 
     //need to delete ip rule here as well!
 
diff -Nurp vyatta-wanloadbalance-0.13.70-old/src/lbtest.cc vyatta-wanloadbalance-0.13.70/src/lbtest.cc
--- vyatta-wanloadbalance-0.13.70-old/src/lbtest.cc	2020-05-15 17:33:53.000000000 +0300
+++ vyatta-wanloadbalance-0.13.70/src/lbtest.cc	2020-07-07 15:01:39.936265697 +0300
@@ -244,7 +244,7 @@ int
 LBTest::receive(int recv_sock)
 {
   timeval wait_time;
-  int icmp_pktsize = 40;
+  int icmp_pktsize = 56;
   char resp_buf[icmp_pktsize];
   icmphdr *icmp_hdr;
   fd_set readfs;
diff -Nurp vyatta-wanloadbalance-old/src/lbdatafactory.cc vyatta-wanloadbalance/src/lbdatafactory.cc
--- vyatta-wanloadbalance-old/src/lbdatafactory.cc	2020-05-31 09:45:24.770000000 +0300
+++ vyatta-wanloadbalance/src/lbdatafactory.cc	2020-06-12 20:48:50.560000000 +0300
@@ -179,6 +179,9 @@ LBDataFactory::process(const vector<stri
     else if (depth == 4 && key == "ttl") {
       process_health_interface_rule_type_udp(l_key,l_value);
     }
+    else if (depth == 4 && key == "port") {
+      process_health_interface_rule_type_udp(l_key,l_value);
+    }
     else if (depth == 4 && key == "test-script") {
       process_health_interface_rule_type_user(l_key,l_value);
     }
@@ -294,7 +297,7 @@ LBDataFactory::process_health_interface(
   else if (key == "health") {
     //nothing
   }
-  else {
+  else if (key != "") {
     if (_debug) {
       cout << "LBDataFactory::process_health(): " << "don't understand this symbol: " << key << endl;
     }

Details

Difficulty level
Normal (likely a few hours)
Version
1.3
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)
Issue type
Bug (incorrect behavior)

Event Timeline

I feel more like abandoning that daemon and use a python based implementation.

If the process is simple, you can consider py script. If you need to run uninterrupted in the background or have other complex tasks, it is a good way to continue to use C/C++

Are you familiar with that codebase @jack9603301? As I see no real answer in your posting which does not help at all :(

I have to say that I saw the diff of this code base for the first time, but in fact, as for my understanding of C troubleshooting, analyzing the log and execution process will become a necessary work (none of them). I can see that the operation process required by this code base may be relatively complex, so that it can be executed as a background daemons rather than a single place According to the description of the questioner, there is hardly any relevant information that can be extracted from the error process. It is suggested to provide the following information:

a) Of course, as stated by @c-po, it is necessary to provide its specific configuration

b) Provide a description of the expected results

c) Provide the actual results, such as error report or routing table policy information, and use the most direct command of Linux to explore what exception occurs?

d) Provide conditions for abnormal recurrence so that testers can conduct relevant tests

e) If possible, provide any generated valuable log information about the service

For @c-po, of course, we can't find the root cause of the error from the question post itself, because the questioner lacks the necessary error exploration, so that developers may need to explore the error points by themselves, or have carried out simple exploration, but the amount of information is insufficient. From the questioner's post, except for a diff, I can't see any valuable information, such as log

I have just quickly browsed the relevant code of this code base. Generally speaking, the code structure is very clear, which is not difficult to understand. The overall operation of this daemon is divided into the following processes:

a) check interface address and nexthop

b) Health testing

c) apply rules

d) update output

These operations are carried out continuously in the loop, and the routing table and related policies of iptables are adjusted according to the predetermined rules. However, the key is that the information provided by the questioner is not enough, so it is impossible to determine what link is wrong?

I don’t understand you, is it really impossible to see from the diff that stupid errors have been fixed here?

You mean you've fixed the bug? If so, you can consider submitting pr

I thought you were asking for help and trying to report a bug to the community for a fix, so you need more information to help identify the problem, rather than a diff who doesn't know where to find it

This is worked patches for me. I suggest including my corrections in the main code

If so, you should make it clear that the diff is your own patch. If you are sure that the patch is available in the latest rolling image, you can consider submitting a PR yourself (please refer to the contributor's Guide) to seek consolidation

@jack9603301 the approach from @banditos13 is perfectly fine. Despite the fact that a PR would be the non plus ultra - a problem was identified and a fix was provided - works for me.

Thank you @banditos13

@c-po Please forgive me that I didn't react until it told me that this is a patch. It has solved the problem. In this case, if it is confirmed that the patch is available, then it is not necessary to discuss the cause of the problem. What we need to do is to apply the patch and conduct relevant tests

Thanks

erkin set Issue type to Bug (incorrect behavior).Sun, Aug 29, 1:50 PM
erkin removed a subscriber: Active contributors.