23 Aug
2012
23 Aug
'12
1:33 p.m.
On Thu, Aug 23, 2012 at 09:19:17AM +0200, Holger Hans Peter Freyther wrote:
This will not help, ret is overwritten by ipaccess_send_*, I need to
reload it anyway for the PING and ID_ACK cases after the send().
I have changed this to:
int ret;
And below, inlined the changes I've done for the new patch version:
If you refer to remove ipa_ccm and to use ret instead, well, it's just
that I have merged ret and ipa_ccm, and ipa_ccm = send(...) seems ugly
to me.
If you refer to the entire function, yes I think it reads better now.
Added case 0 (this is not an IPA control message) and default case
using LOGP to report an error.
I've changed all send callers to check for ret != msg->len. Note that
if we ever support TCP segmentation (TCP_NODELAY unset), then we'll
have to relax that checking (although I don't think we'll ever do,
unless for the ipaccess BTS).
Thanks for you review Holger.
On Wed, Aug 22, 2012 at 05:04:48PM +0200,
pablo(a)gnumonks.org wrote:
No problem, I'll fix the description.
From: Pablo Neira Ayuso
<pablo(a)gnumonks.org>
In case of problems to transmit message, spot an error and
close the socket to force a re-establishment.
This also modifies ipa_client_read to close the socket in
case that our callback reports an error.
this somehow doesn't read right. Sorry to be picky here. :} + LOGP(DLINP, LOGL_ERROR, "connection
closed with "
+ "server\n");
I try to have full sentences in the log message. This means it starts
with a capital letter and ends with a full stop.
int ret = 0;
int ret = 1 ; >
> switch (msg_type) {
> case IPAC_MSGT_PING:
> - ipa_ccm = 1;
> ret = ipaccess_send_pong(bfd->fd);
> + if (ret < 0) {
> + LOGP(DLINP, LOGL_ERROR, "cannot send PING "
> + "message. Reason: %s\n", strerror(errno));
break;
}
ret = 1;
> break;
> case IPAC_MSGT_PONG:
> DEBUGP(DLMI, "PONG!\n");
> break;
> case IPAC_MSGT_ID_ACK:
> DEBUGP(DLMI, "ID_ACK? -> ACK!\n");
> - ipa_ccm = 1;
> ret = ipaccess_send_id_ack(bfd->fd);
> + if (ret < 0) {
> + LOGP(DLINP, LOGL_ERROR, "cannot send ID_ACK "
> + "message. Reason: %s\n", strerror(errno));
break;
}
ret = 1;
break;
default:
ret = 0;
break;
}
- return ipa_ccm;
+ return ret;
}
does this read any better? static int
ipaccess_drop(struct osmo_fd *bfd)
@@ -269,8 +280,15 @@ static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb
*msg,
int len, ret;
/* Handle IPA PING, PONG and ID_ACK messages. */
- if (ipaccess_rcvmsg_base(msg, bfd))
+ ret = ipaccess_rcvmsg_base(msg, bfd);
+ switch(ret) {
+ case -1:
+ /* error in IPA control message handling */
+ goto err;
+ case 1:
+ /* this is an IPA control message, skip further processing */
return 0;
default:
WARN_ON(ret);
? + }
switch (msg_type) {
case IPAC_MSGT_ID_RESP:
@@ -517,6 +535,16 @@ static int __handle_ts1_write(struct osmo_fd *bfd, struct e1inp_line
*line)
DEBUGP(DLMI, "TX %u: %s\n", ts_nr, osmo_hexdump(msg->l2h,
msgb_l2len(msg)));
ret = send(bfd->fd, msg->data, msg->len, 0);
+ if (ret < 0) {
if (ret != msg->len)